Skip to content
Permalink
Browse files

[JENKINS-22938] SSH slave connections die after the slave outputs 4MB…

… of stderr, usually during findbugs analysis

The fix for [JENKINS-18836], [JENKINS-18879], [JENKINS-19619] was incorrect in its analysis.

- There is no call to getChannelData() on the new code path, so thus you cannot have two calls of freeupWindow()
- The problem with the original call to freeupWindow() is that it is on the receiver thread. You should not mix the responsibilities. Blocking the receiver thread to send a message will negatively impact performance and connection stability.
- The correct solution is to push the freeupWindow onto the async queue thus the ACK gets sent and the purity of the receiving thread can be maintained.
  • Loading branch information...
stephenc committed May 14, 2014
1 parent f0712eb commit 5811ddd7ae15670a4f9ad345352613b3f2f2db97
Showing with 16 additions and 2 deletions.
  1. +16 −2 src/com/trilead/ssh2/channel/Channel.java
@@ -96,6 +96,7 @@ public void write(byte[] buf, int start, int len) throws IOException {
}
} else {
sink.write(buf,start,len);
freeupWindow(len, true);
}
}

@@ -327,6 +328,14 @@ public void setReasonClosed(String reasonClosed)
* let it send more data.
*/
void freeupWindow(int copylen) throws IOException {
freeupWindow(copylen, false);
}

/**
* Update the flow control couner and if necessary, sends ACK to the other end to
* let it send more data.
*/
void freeupWindow(int copylen, boolean sendAsync) throws IOException {
if (copylen <= 0) return;

int increment = 0;
@@ -375,8 +384,13 @@ void freeupWindow(int copylen) throws IOException {
msg[7] = (byte) (increment >> 8);
msg[8] = (byte) (increment);

if (closeMessageSent == false)
cm.tm.sendMessage(msg);
if (closeMessageSent == false) {
if (sendAsync) {
cm.tm.sendAsynchronousMessage(msg);
} else {
cm.tm.sendMessage(msg);
}
}
}
}
}

0 comments on commit 5811ddd

Please sign in to comment.
You can’t perform that action at this time.