Skip to content

Commit

Permalink
Add a getChildProcessId method to WinPty and WinPtyProcess.
Browse files Browse the repository at this point in the history
Ensure that the process handle stays open until both of these conditions
are met:
 1. The process has exited.
 2. We have disconnected from the agent, by closing the winpty_t
    object.

For consistency with the getChildProcessId method in pty4j-0.6, and to
reduce the possibility of a race condition, have getChildProcessId return
-1 as soon as the WinPty object is closed, even if the process is still
running (for a brief moment).

Rename Reaper to WaitForExitThread.  I think the "reaper" on Unix is
so-named because it cleans up zombie processes.  The WinPty equivalent
instead leaves a zombie process handle/PID alive while until the WinPty
object is closed.
  • Loading branch information
rprichard committed May 31, 2016
1 parent b3728b9 commit d757ecd
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 18 deletions.
68 changes: 50 additions & 18 deletions src/com/pty4j/windows/WinPty.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@
import java.io.IOException;

import static com.sun.jna.platform.win32.WinBase.INFINITE;
import static com.sun.jna.platform.win32.WinBase.WAIT_OBJECT_0;
import static com.sun.jna.platform.win32.WinNT.GENERIC_READ;
import static com.sun.jna.platform.win32.WinNT.GENERIC_WRITE;

/**
* @author traff
*/
public class WinPty {
private final Pointer myWinpty;
private Pointer myWinpty;

private WinNT.HANDLE myProcess = null;
private NamedPipe myConinPipe;
private NamedPipe myConoutPipe;
private NamedPipe myConerrPipe;

private boolean myDone = false;
private boolean myChildExited = false;
private int myStatus = -1;
private boolean myClosed = false;

Expand Down Expand Up @@ -88,15 +88,15 @@ public WinPty(String cmdline, String cwd, String env, boolean consoleMode) throw

// Success! Save the values we want and let the `finally` block clean up the rest.

// Designate a thread to wait for the process to exit. That thread will
// also be responsible for closing the process handle.
new Reaper(processHandle.getValue()).start();

myWinpty = winpty;
myProcess = processHandle.getValue();
myConinPipe = coninPipe;
myConoutPipe = conoutPipe;
myConerrPipe = conerrPipe;

// Designate a thread to wait for the process to exit.
new WaitForExitThread().start();

winpty = null;
processHandle.setValue(null);
coninPipe = conoutPipe = conerrPipe = null;
Expand Down Expand Up @@ -135,27 +135,57 @@ public synchronized void setWinSize(WinSize winSize) {
INSTANCE.winpty_set_size(myWinpty, winSize.ws_col, winSize.ws_row, null);
}

// Close the winpty_t object, which disconnects libwinpty from the winpty
// agent process. The agent will then close the hidden console, killing
// everything attached to it.
public synchronized void close() {
if (myClosed) {
return;
}
INSTANCE.winpty_free(myWinpty);
myWinpty = null;
myClosed = true;
closeUnusedProcessHandle();
}

private synchronized void closeUnusedProcessHandle() {
// Keep the process handle open until both conditions are met:
// 1. The process has exited.
// 2. We have disconnected from the agent, by closing the winpty_t
// object.
// As long as the process handle is open, Windows will not reuse the child
// process' PID.
// https://blogs.msdn.microsoft.com/oldnewthing/20110107-00/?p=11803
if (myClosed && myChildExited && myProcess != null) {
Kernel32.INSTANCE.CloseHandle(myProcess);
myProcess = null;
}
}

// Returns true if the child process is still running. The winpty_t and
// WinPty objects may be closed/freed either before or after the child
// process exits.
public synchronized boolean isRunning() {
return !myDone;
return !myChildExited;
}

// Waits for the child process to exit.
public synchronized int waitFor() throws InterruptedException {
while (!myDone) {
while (!myChildExited) {
wait();
}
return myStatus;
}

public synchronized int getChildProcessId() {
if (myClosed) {
return -1;
}
return Kernel32.INSTANCE.GetProcessId(myProcess);
}

public synchronized int exitValue() {
if (!myDone) {
if (!myChildExited) {
throw new IllegalThreadStateException("Process not Terminated");
}
return myStatus;
Expand Down Expand Up @@ -188,24 +218,26 @@ public NamedPipe getErrorPipe() {
return myConerrPipe;
}

private class Reaper extends Thread {
private WinNT.HANDLE myProcess;
// It is mostly possible to avoid using this thread; instead, the above
// methods could call WaitForSingleObject themselves, using either a 0 or
// INFINITE timeout as appropriate. It is tricky, though, because we need
// to avoid closing the process handle as long as any threads are waiting on
// it, but we can't do an INFINITE wait inside a synchronized method. It
// could be done using an extra reference count, or by using DuplicateHandle
// for INFINITE waits.
private class WaitForExitThread extends Thread {
private IntByReference myStatusByRef = new IntByReference(-1);

Reaper(WinNT.HANDLE process) {
myProcess = process;
}

@Override
public void run() {
Kernel32.INSTANCE.WaitForSingleObject(myProcess, INFINITE);
Kernel32.INSTANCE.GetExitCodeProcess(myProcess, myStatusByRef);
synchronized (WinPty.this) {
WinPty.this.myDone = true;
WinPty.this.myChildExited = true;
WinPty.this.myStatus = myStatusByRef.getValue();
closeUnusedProcessHandle();
WinPty.this.notifyAll();
}
Kernel32.INSTANCE.CloseHandle(myProcess);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/com/pty4j/windows/WinPtyProcess.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ public int waitFor() throws InterruptedException {
return myWinPty.waitFor();
}

public int getChildProcessId() {
return myWinPty.getChildProcessId();
}

@Override
public int exitValue() {
return myWinPty.exitValue();
Expand Down

0 comments on commit d757ecd

Please sign in to comment.