Windows processes spawned by NuProcess may deadlock an application when processes are spawned from different threads

[tipyramid]

Hi,

I've encountered a scenario where running opening two different processes in parallel (i.e different threads) can cause a scenario where one child process spawned by NuProcess, blocks another process exit signal from being received in time by NuProcess, causing any thread that waits for the blocked process on NuProcess.waitFor() to wait until the first one closes.

This issue happens on Windows. I haven't properly tested this on other OSes, but the trigger for it was seen on our systems on Windows.

Consider for example where you want to run the following two commands:

1. powershell.exe -Command "echo hello"
2. powershell.exe -Command "Start-Sleep -Seconds 50"

I would start command 1 in Thread A, and command 2 in Thread B, as such

// Define a process handler to print the standard output of the child processes
class MyProcessHandler extends NuAbstractProcessHandler {
	@Override
	public void onStart(NuProcess nuProcess) {
	}

	@Override
	public boolean onStdinReady(ByteBuffer buffer) {
		return false; // false means we have nothing else to write at this time
	}

	@Override
	public void onStdout(ByteBuffer buffer, boolean closed) {
		if (!closed) {
			byte[] bytes = new byte[buffer.remaining()];
			buffer.get(bytes);
			System.out.println(new String(bytes));
		}
	}
}

// In some method:
Thread tA = new Thread(() -> {
    NuProcessBuilder nuProcessbuilder = new NuProcessBuilder(new MyProcessHandler(), "powershell.exe", "-Command", "echo hello");
    NuProcess process = nuProcessbuilder.start();
    try {
        process.waitFor(0, TimeUnit.DAYS);
    } catch (InterruptedException e) {
        e.printStackTrace();
        throw new RuntimeException(e);
    }
});

Thread tB = new Thread(() -> {
    NuProcessBuilder nuProcessbuilder = new NuProcessBuilder(new MyProcessHandler(), "powershell.exe", "-Command", "Start-Sleep -Seconds 50");
    NuProcess process = nuProcessbuilder.start();
    try {
        process.waitFor(0, TimeUnit.DAYS);
    } catch (InterruptedException e) {
        e.printStackTrace();
        throw new RuntimeException(e);
    }
});

tA.start();
tB.start();

Then I want to wait for the first process to finish, and afterwards, I would wait for the second one:

tA.join();
System.out.println("Process 1 finished");
tB.join();
System.out.println("Process 2 finished");

In this scenario, it turns out the Process 1 will not return after Process 2 has returned. In our case 50 seconds.
Worse can happen is if Process 2 slept forever. that would cause tA.join(); to never return.

This happens because of named pipe handle inheritance from the parent process.
From what I gathered by debugging the issue locally, during the call to WindowsProcess.start() the following happens (in separate calls to private methods):

1. Named pipes are created for stdin, stdout and stderr for the Process in question, stored in hStdinWidow, hStdoutWidow, and hStderrWidow respectively. The pipes are created with bInheritHandle = true set on the SECURITY_ATTRIBUTES securityAttributes parameter.
2. The parent java process connects to those pipes as the Reading end for stdout/stderr and Writing end for stdin.
3. A child process is then created, with stdin, stdout and stderr attached to the handles created initially (hStdinWidow, hStdoutWidow, and hStderrWidow) with bInheritHandles = true.
4. Finally, at the end of WindowsProcess.start(), the handles are closed on the parent process' side.

At this point, the child process has inherited (a duplicate) of the handles and can write to them.

So what happens when both threads run in parallel during execution of WindowsProcess.start()
It seems that, before the handles are closed on the parent process' side, they are inherited for both processes.
So, even though each child process will only use their attached handles, they would still inherit a copy of the handles for the pipes created for the other process.

Now, how does this cause process 2 to block process 1:
Since threads running ProcessCompletions.run() periodically check for IOCP queue completion messages, eventually a message should arrive indicating that the stdout/stderr pipes have closed, leading NuProcess to signal any waiting threads on WindowsProcess.waitFor().

Even if process 1 has closed, its "pipe closed" message (failed messages with error code 109), would not arrive because copies of the pipe handles are still held by process 2.

Once process 2 exits (or killed), the handles it held will be closed by the OS, causing the OS to send the messages for both processes on the IOCP queue.
In the example above, this will lead to a signal to arrive on tA.join(), releasing the waiting thread.

The following post by Raymond Chen describes the inheritance problem and the solution:
https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873

The win32 api functions described can let you limit the inheritance of each of the name pipes' handles corresponding to their related child processes, such that process 1 would inherit its handles only and process 2 would inherit only theirs - assuming we want to limit the inheritance of all other handles in the parent process (eg. open files handles)...

I have made an implementation on my end to verify that this works, though It's quite quirky and I don't have much experience with JNA myself. So I thought that it'd probably be better to open an issue instead and describe my findings thoroughly.

Thanks