Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix zombie Input Reader thread when user changes password #169

Merged
merged 1 commit into from
Mar 14, 2016

Conversation

gschueler
Copy link
Contributor

A client was having a similar issue to previous #87 (using overthere 4.0.0 via rundeck winrm plugin, the problem exists in latest code as well).

The command they are executing changes the user's own password, and that causes the WinRM connection to be dropped immediately. It seems it leaves the overthere connection in a weird state, with the input reader thread still alive. After many executions of this, there were a lot of zombie "WinRM input reader for command..." threads in the JVM.

Here is the thread dump of the lingering threads:

"WinRM input reader for command [8852E115-51DD-4906-90E2-1B88C616A5B4]" - 
Thread t@141
java.lang.Thread.State: TIMED_WAITING
at java.lang.Object.wait(Native Method)
- waiting on <cf9004f> (a java.io.PipedInputStream)
at java.io.PipedInputStream.read(PipedInputStream.java:327)
at java.io.PipedInputStream.read(PipedInputStream.java:378)
at java.io.InputStream.read(InputStream.java:101)
at 
com.xebialabs.overthere.cifs.winrm.CifsWinRmConnection$1.run(CifsWinRmConnection.java:130)

I think the problem is in the finally clause of the OverthereProcess.waitFor() created in CifsWinRmConnection.java#startProcess:

try {

    outputReaderThread.join();

} finally {

    winRmClient.deleteShell(); //might throw exception

    closeQuietly(callersStdin);

    processTerminated = true;

}

If the authentication is no longer valid, any call to winRmClient is going to throw an exception, but the finally clause calls deleteShell prior to stopping the thread for Stdin handling.

The outputReaderThread calls winRmClient.receiveOutput, which I think causes an IO exception when the authentication is no longer valid. I.e. the winrmClient.sendRequest method will throw an exception due to authentication failure.

At this point the outputReaderThread will finish, and the .join() call will complete.

However the finally clause calls winRmClient.deleteShell(), which also tries to do winrmClient.sendRequest, throwing another exception. The closeQuietly is never performed, letting the thread reading the PipedInputStream stay alive.

The client tested using this patch, and did not see any more zombie threads.

(Aside: The method BaseOverthereConnection.execute, does not provide any way to either use or close the stdin to the command, which may have been a workaround for us, and since it is not used it seems it probably should close it automatically.)

vpartington pushed a commit that referenced this pull request Mar 14, 2016
Fix zombie Input Reader thread when user changes password
@vpartington vpartington merged commit d633d8d into xebialabs:master Mar 14, 2016
@vpartington
Copy link
Contributor

@gschueler, thank you for this PR. I'll soon create a new release of Overthere that will include this fix and other. As well as support for HTTP and SOCKS proxies as jumpstations (see #170).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants