Prevent unexpected native controller output hanging the process (#56685)
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 Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
parent
b98b260048
commit
ab40466bfb
|
@ -91,7 +91,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
|
||||||
"has.native.controller", "false");
|
"has.native.controller", "false");
|
||||||
|
|
||||||
try (Spawner spawner = new Spawner()) {
|
try (Spawner spawner = new Spawner()) {
|
||||||
spawner.spawnNativeControllers(environment);
|
spawner.spawnNativeControllers(environment, false);
|
||||||
assertThat(spawner.getProcesses(), hasSize(0));
|
assertThat(spawner.getProcesses(), hasSize(0));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -149,7 +149,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
|
||||||
"has.native.controller", "false");
|
"has.native.controller", "false");
|
||||||
|
|
||||||
Spawner spawner = new Spawner();
|
Spawner spawner = new Spawner();
|
||||||
spawner.spawnNativeControllers(environment);
|
spawner.spawnNativeControllers(environment, false);
|
||||||
|
|
||||||
List<Process> processes = spawner.getProcesses();
|
List<Process> processes = spawner.getProcesses();
|
||||||
|
|
||||||
|
@ -196,7 +196,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
|
||||||
Spawner spawner = new Spawner();
|
Spawner spawner = new Spawner();
|
||||||
IllegalArgumentException e = expectThrows(
|
IllegalArgumentException e = expectThrows(
|
||||||
IllegalArgumentException.class,
|
IllegalArgumentException.class,
|
||||||
() -> spawner.spawnNativeControllers(environment));
|
() -> spawner.spawnNativeControllers(environment, false));
|
||||||
assertThat(
|
assertThat(
|
||||||
e.getMessage(),
|
e.getMessage(),
|
||||||
equalTo("module [test_plugin] does not have permission to fork native controller"));
|
equalTo("module [test_plugin] does not have permission to fork native controller"));
|
||||||
|
@ -217,10 +217,10 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
|
||||||
final Spawner spawner = new Spawner();
|
final Spawner spawner = new Spawner();
|
||||||
if (Constants.MAC_OS_X) {
|
if (Constants.MAC_OS_X) {
|
||||||
// if the spawner were not skipping the Desktop Services Store files on macOS this would explode
|
// if the spawner were not skipping the Desktop Services Store files on macOS this would explode
|
||||||
spawner.spawnNativeControllers(environment);
|
spawner.spawnNativeControllers(environment, false);
|
||||||
} else {
|
} else {
|
||||||
// we do not ignore these files on non-macOS systems
|
// we do not ignore these files on non-macOS systems
|
||||||
final FileSystemException e = expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment));
|
final FileSystemException e = expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment, false));
|
||||||
if (Constants.WINDOWS) {
|
if (Constants.WINDOWS) {
|
||||||
assertThat(e, instanceOf(NoSuchFileException.class));
|
assertThat(e, instanceOf(NoSuchFileException.class));
|
||||||
} else {
|
} else {
|
||||||
|
|
|
@ -170,7 +170,7 @@ final class Bootstrap {
|
||||||
Settings settings = environment.settings();
|
Settings settings = environment.settings();
|
||||||
|
|
||||||
try {
|
try {
|
||||||
spawner.spawnNativeControllers(environment);
|
spawner.spawnNativeControllers(environment, true);
|
||||||
} catch (IOException e) {
|
} catch (IOException e) {
|
||||||
throw new BootstrapException(e);
|
throw new BootstrapException(e);
|
||||||
}
|
}
|
||||||
|
|
|
@ -55,10 +55,12 @@ final class Spawner implements Closeable {
|
||||||
/**
|
/**
|
||||||
* Spawns the native controllers for each module.
|
* Spawns the native controllers for each module.
|
||||||
*
|
*
|
||||||
* @param environment the node environment
|
* @param environment The node environment
|
||||||
|
* @param inheritIo Should the stdout and stderr of the spawned process inherit the
|
||||||
|
* stdout and stderr of the JVM spawning it?
|
||||||
* @throws IOException if an I/O error occurs reading the module or spawning a native process
|
* @throws IOException if an I/O error occurs reading the module or spawning a native process
|
||||||
*/
|
*/
|
||||||
void spawnNativeControllers(final Environment environment) throws IOException {
|
void spawnNativeControllers(final Environment environment, final boolean inheritIo) throws IOException {
|
||||||
if (!spawned.compareAndSet(false, true)) {
|
if (!spawned.compareAndSet(false, true)) {
|
||||||
throw new IllegalStateException("native controllers already spawned");
|
throw new IllegalStateException("native controllers already spawned");
|
||||||
}
|
}
|
||||||
|
@ -83,7 +85,7 @@ final class Spawner implements Closeable {
|
||||||
modules.getFileName());
|
modules.getFileName());
|
||||||
throw new IllegalArgumentException(message);
|
throw new IllegalArgumentException(message);
|
||||||
}
|
}
|
||||||
final Process process = spawnNativeController(spawnPath, environment.tmpFile());
|
final Process process = spawnNativeController(spawnPath, environment.tmpFile(), inheritIo);
|
||||||
processes.add(process);
|
processes.add(process);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -92,7 +94,7 @@ final class Spawner implements Closeable {
|
||||||
* Attempt to spawn the controller daemon for a given module. The spawned process will remain connected to this JVM via its stdin,
|
* Attempt to spawn the controller daemon for a given module. The spawned process will remain connected to this JVM via its stdin,
|
||||||
* stdout, and stderr streams, but the references to these streams are not available to code outside this package.
|
* stdout, and stderr streams, but the references to these streams are not available to code outside this package.
|
||||||
*/
|
*/
|
||||||
private Process spawnNativeController(final Path spawnPath, final Path tmpPath) throws IOException {
|
private Process spawnNativeController(final Path spawnPath, final Path tmpPath, final boolean inheritIo) throws IOException {
|
||||||
final String command;
|
final String command;
|
||||||
if (Constants.WINDOWS) {
|
if (Constants.WINDOWS) {
|
||||||
/*
|
/*
|
||||||
|
@ -114,6 +116,14 @@ final class Spawner implements Closeable {
|
||||||
pb.environment().clear();
|
pb.environment().clear();
|
||||||
pb.environment().put("TMPDIR", tmpPath.toString());
|
pb.environment().put("TMPDIR", tmpPath.toString());
|
||||||
|
|
||||||
|
// The process _shouldn't_ write any output via its stdout or stderr, but if it does then
|
||||||
|
// it will block if nothing is reading that output. To avoid this we can inherit the
|
||||||
|
// JVM's stdout and stderr (which are redirected to files in standard installations).
|
||||||
|
if (inheritIo) {
|
||||||
|
pb.redirectOutput(ProcessBuilder.Redirect.INHERIT);
|
||||||
|
pb.redirectError(ProcessBuilder.Redirect.INHERIT);
|
||||||
|
}
|
||||||
|
|
||||||
// the output stream of the process object corresponds to the daemon's stdin
|
// the output stream of the process object corresponds to the daemon's stdin
|
||||||
return pb.start();
|
return pb.start();
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue