Skip to content

Commit 875cfee

Browse files
committed
Revert "Prevent unexpected native controller output hanging the process (#56688)"
This reverts commit dec7c21.
1 parent 10c4ab6 commit 875cfee

File tree

3 files changed

+10
-20
lines changed

3 files changed

+10
-20
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, false);
94+
spawner.spawnNativeControllers(environment);
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, false);
152+
spawner.spawnNativeControllers(environment);
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, false));
199+
() -> spawner.spawnNativeControllers(environment));
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, false);
220+
spawner.spawnNativeControllers(environment);
221221
} else {
222222
// we do not ignore these files on non-macOS systems
223-
final FileSystemException e = expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment, false));
223+
final FileSystemException e = expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment));
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, true);
165+
spawner.spawnNativeControllers(environment);
166166
} catch (IOException e) {
167167
throw new BootstrapException(e);
168168
}

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

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,10 @@ public void close() throws IOException {
5555
/**
5656
* Spawns the native controllers for each module.
5757
*
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?
58+
* @param environment the node environment
6159
* @throws IOException if an I/O error occurs reading the module or spawning a native process
6260
*/
63-
void spawnNativeControllers(final Environment environment, final boolean inheritIo) throws IOException {
61+
void spawnNativeControllers(final Environment environment) throws IOException {
6462
if (!spawned.compareAndSet(false, true)) {
6563
throw new IllegalStateException("native controllers already spawned");
6664
}
@@ -85,7 +83,7 @@ void spawnNativeControllers(final Environment environment, final boolean inherit
8583
modules.getFileName());
8684
throw new IllegalArgumentException(message);
8785
}
88-
final Process process = spawnNativeController(spawnPath, environment.tmpFile(), inheritIo);
86+
final Process process = spawnNativeController(spawnPath, environment.tmpFile());
8987
processes.add(process);
9088
}
9189
}
@@ -94,7 +92,7 @@ void spawnNativeControllers(final Environment environment, final boolean inherit
9492
* Attempt to spawn the controller daemon for a given module. The spawned process will remain connected to this JVM via its stdin,
9593
* stdout, and stderr streams, but the references to these streams are not available to code outside this package.
9694
*/
97-
private Process spawnNativeController(final Path spawnPath, final Path tmpPath, final boolean inheritIo) throws IOException {
95+
private Process spawnNativeController(final Path spawnPath, final Path tmpPath) throws IOException {
9896
final String command;
9997
if (Constants.WINDOWS) {
10098
/*
@@ -116,14 +114,6 @@ private Process spawnNativeController(final Path spawnPath, final Path tmpPath,
116114
pb.environment().clear();
117115
pb.environment().put("TMPDIR", tmpPath.toString());
118116

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-
127117
// the output stream of the process object corresponds to the daemon's stdin
128118
return pb.start();
129119
}

0 commit comments

Comments
 (0)