Skip to content

Commit dec7c21

Browse files
authored
Prevent unexpected native controller output hanging the process (#56688)
In normal operation native controllers are not expected to write anything to stdout or stderr. However, if due to an error or something unexpected with the environment a native controller does write something to stdout or stderr then it will block if nothing is reading that output. This change makes the stdout and stderr of native controllers reuse the same stdout and stderr as the Elasticsearch JVM (which are by default redirected to es.stdout.log and es.stderr.log) so that if something unexpected is written to native controller output then: 1. The native controller process does not block, waiting for something to read the output 2. We can see what the output was, making it easier to debug obscure environmental problems Backport of #56491
1 parent b237f57 commit dec7c21

File tree

3 files changed

+20
-10
lines changed

3 files changed

+20
-10
lines changed

qa/no-bootstrap-tests/src/test/java/org/elasticsearch/bootstrap/SpawnerNoBootstrapTests.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public void testNoControllerSpawn() throws IOException {
9191
"has.native.controller", "false");
9292

9393
try (Spawner spawner = new Spawner()) {
94-
spawner.spawnNativeControllers(environment);
94+
spawner.spawnNativeControllers(environment, false);
9595
assertThat(spawner.getProcesses(), hasSize(0));
9696
}
9797
}
@@ -149,7 +149,7 @@ private void assertControllerSpawns(final Function<Environment, Path> pluginsDir
149149
"has.native.controller", "false");
150150

151151
Spawner spawner = new Spawner();
152-
spawner.spawnNativeControllers(environment);
152+
spawner.spawnNativeControllers(environment, false);
153153

154154
List<Process> processes = spawner.getProcesses();
155155

@@ -196,7 +196,7 @@ public void testControllerSpawnWithIncorrectDescriptor() throws IOException {
196196
Spawner spawner = new Spawner();
197197
IllegalArgumentException e = expectThrows(
198198
IllegalArgumentException.class,
199-
() -> spawner.spawnNativeControllers(environment));
199+
() -> spawner.spawnNativeControllers(environment, false));
200200
assertThat(
201201
e.getMessage(),
202202
equalTo("module [test_plugin] does not have permission to fork native controller"));
@@ -217,10 +217,10 @@ public void testSpawnerHandlingOfDesktopServicesStoreFiles() throws IOException
217217
final Spawner spawner = new Spawner();
218218
if (Constants.MAC_OS_X) {
219219
// if the spawner were not skipping the Desktop Services Store files on macOS this would explode
220-
spawner.spawnNativeControllers(environment);
220+
spawner.spawnNativeControllers(environment, false);
221221
} else {
222222
// we do not ignore these files on non-macOS systems
223-
final FileSystemException e = expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment));
223+
final FileSystemException e = expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment, false));
224224
if (Constants.WINDOWS) {
225225
assertThat(e, instanceOf(NoSuchFileException.class));
226226
} else {

server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ private void setup(boolean addShutdownHook, Environment environment) throws Boot
162162
Settings settings = environment.settings();
163163

164164
try {
165-
spawner.spawnNativeControllers(environment);
165+
spawner.spawnNativeControllers(environment, true);
166166
} catch (IOException e) {
167167
throw new BootstrapException(e);
168168
}

server/src/main/java/org/elasticsearch/bootstrap/Spawner.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,12 @@ public void close() throws IOException {
5555
/**
5656
* Spawns the native controllers for each module.
5757
*
58-
* @param environment the node environment
58+
* @param environment The node environment
59+
* @param inheritIo Should the stdout and stderr of the spawned process inherit the
60+
* stdout and stderr of the JVM spawning it?
5961
* @throws IOException if an I/O error occurs reading the module or spawning a native process
6062
*/
61-
void spawnNativeControllers(final Environment environment) throws IOException {
63+
void spawnNativeControllers(final Environment environment, final boolean inheritIo) throws IOException {
6264
if (!spawned.compareAndSet(false, true)) {
6365
throw new IllegalStateException("native controllers already spawned");
6466
}
@@ -83,7 +85,7 @@ void spawnNativeControllers(final Environment environment) throws IOException {
8385
modules.getFileName());
8486
throw new IllegalArgumentException(message);
8587
}
86-
final Process process = spawnNativeController(spawnPath, environment.tmpFile());
88+
final Process process = spawnNativeController(spawnPath, environment.tmpFile(), inheritIo);
8789
processes.add(process);
8890
}
8991
}
@@ -92,7 +94,7 @@ void spawnNativeControllers(final Environment environment) throws IOException {
9294
* Attempt to spawn the controller daemon for a given module. The spawned process will remain connected to this JVM via its stdin,
9395
* stdout, and stderr streams, but the references to these streams are not available to code outside this package.
9496
*/
95-
private Process spawnNativeController(final Path spawnPath, final Path tmpPath) throws IOException {
97+
private Process spawnNativeController(final Path spawnPath, final Path tmpPath, final boolean inheritIo) throws IOException {
9698
final String command;
9799
if (Constants.WINDOWS) {
98100
/*
@@ -114,6 +116,14 @@ private Process spawnNativeController(final Path spawnPath, final Path tmpPath)
114116
pb.environment().clear();
115117
pb.environment().put("TMPDIR", tmpPath.toString());
116118

119+
// The process _shouldn't_ write any output via its stdout or stderr, but if it does then
120+
// it will block if nothing is reading that output. To avoid this we can inherit the
121+
// JVM's stdout and stderr (which are redirected to files in standard installations).
122+
if (inheritIo) {
123+
pb.redirectOutput(ProcessBuilder.Redirect.INHERIT);
124+
pb.redirectError(ProcessBuilder.Redirect.INHERIT);
125+
}
126+
117127
// the output stream of the process object corresponds to the daemon's stdin
118128
return pb.start();
119129
}

0 commit comments

Comments
 (0)