Skip to content

Commit

Permalink
Adding timeouts to wait methods, to never allow for infinite wait() s…
Browse files Browse the repository at this point in the history
…tate. (#198)

* Adding timeouts to wait methods, to never allow for infinite thread wait.

* Update StreamGobbler.java

Add timeouts to StreamGobbler methods

* Update ChannelManager.java

Use existing DEFAULT_WAIT_TIMEOUT for timeout

* Update TransportManager.java

Add DEFAULT_WAIT_TIMEOUT, and use it for wait() calls

* Update KexManager.java

* Update FifoBuffer.java

* Increase default timeout to 20min, to allow for temporary network issues.

* Fix DM_BOXED_PRIMITIVE_FOR_PARSING
  • Loading branch information
Experrior authored Aug 27, 2024
1 parent 15dfef3 commit 15622af
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 11 deletions.
8 changes: 6 additions & 2 deletions src/com/trilead/ssh2/StreamGobbler.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@

public class StreamGobbler extends InputStream
{

private static final String PROPERTY_TIMEOUT = StreamGobbler.class.getName() + ".timeout";
private static long DEFAULT_WAIT_TIMEOUT = Long.parseLong(System.getProperty(PROPERTY_TIMEOUT,"1200000"));

class GobblerThread extends Thread
{
public void run()
Expand Down Expand Up @@ -141,7 +145,7 @@ public int read() throws IOException

try
{
synchronizer.wait();
synchronizer.wait(DEFAULT_WAIT_TIMEOUT);
}
catch (InterruptedException e)
{
Expand Down Expand Up @@ -210,7 +214,7 @@ public int read(byte[] b, int off, int len) throws IOException

try
{
synchronizer.wait();
synchronizer.wait(DEFAULT_WAIT_TIMEOUT);
}
catch (InterruptedException e)
{
Expand Down
2 changes: 1 addition & 1 deletion src/com/trilead/ssh2/auth/AuthenticationManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
public class AuthenticationManager implements MessageHandler
{
public static final String PROPERTY_TIMEOUT = AuthenticationManager.class.getName() + ".timeout";
public static final long TIMEOUT = Long.valueOf(System.getProperty(PROPERTY_TIMEOUT,"120000"));
public static final long TIMEOUT = Long.parseLong(System.getProperty(PROPERTY_TIMEOUT,"1200000"));
TransportManager tm;

Vector packets = new Vector();
Expand Down
6 changes: 3 additions & 3 deletions src/com/trilead/ssh2/channel/ChannelManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class ChannelManager implements MessageHandler
{
private static final Logger log = Logger.getLogger(ChannelManager.class);
private static final String PROPERTY_TIMEOUT = ChannelManager.class.getName() + ".timeout";
private static long DEFAULT_WAIT_TIMEOUT = Long.valueOf(System.getProperty(PROPERTY_TIMEOUT,"120000"));
private static long DEFAULT_WAIT_TIMEOUT = Long.parseLong(System.getProperty(PROPERTY_TIMEOUT,"1200000"));

private HashMap x11_magic_cookies = new HashMap();

Expand Down Expand Up @@ -382,7 +382,7 @@ public void sendData(Channel c, byte[] buffer, int pos, int len) throws IOExcept

try
{
c.wait();
c.wait(DEFAULT_WAIT_TIMEOUT);
}
catch (InterruptedException ignore)
{
Expand Down Expand Up @@ -913,7 +913,7 @@ public int waitForCondition(Channel c, long timeout, int condition_mask) throws
if (timeout > 0)
c.wait(timeout);
else
c.wait();
c.wait(DEFAULT_WAIT_TIMEOUT);
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/com/trilead/ssh2/channel/FifoBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
* @author Kohsuke Kawaguchi
*/
class FifoBuffer {

private static final String PROPERTY_TIMEOUT = FifoBuffer.class.getName() + ".timeout";
private static long DEFAULT_WAIT_TIMEOUT = Long.parseLong(System.getProperty(PROPERTY_TIMEOUT,"1200000"));

/**
* Unit of buffer, singly linked and lazy created as needed.
*/
Expand Down Expand Up @@ -153,7 +157,7 @@ public void write(byte[] buf, int start, int len) throws InterruptedException {

synchronized (lock) {
while ((chunk = Math.min(len,writable()))==0)
lock.wait();
lock.wait(DEFAULT_WAIT_TIMEOUT);

w.write(buf, start, chunk);

Expand Down Expand Up @@ -209,7 +213,7 @@ public int read(byte[] buf, int start, int len) throws InterruptedException {
releaseRing();
return -1; // no more data
}
lock.wait(); // wait until the writer gives us something
lock.wait(DEFAULT_WAIT_TIMEOUT); // wait until the writer gives us something
}

r.read(buf,start,chunk);
Expand Down
4 changes: 3 additions & 1 deletion src/com/trilead/ssh2/transport/KexManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
public class KexManager implements MessageHandler
{
private static final Logger log = Logger.getLogger(KexManager.class);
private static final String PROPERTY_TIMEOUT = KexManager.class.getName() + ".timeout";
private static long DEFAULT_WAIT_TIMEOUT = Long.parseLong(System.getProperty(PROPERTY_TIMEOUT,"1200000"));

private static final List<String> DEFAULT_KEY_ALGORITHMS = buildDefaultKeyAlgorithms();

Expand Down Expand Up @@ -98,7 +100,7 @@ public ConnectionInfo getOrWaitForConnectionInfo(int minKexCount) throws IOExcep

try
{
accessLock.wait();
accessLock.wait(DEFAULT_WAIT_TIMEOUT);
}
catch (InterruptedException e)
{
Expand Down
6 changes: 4 additions & 2 deletions src/com/trilead/ssh2/transport/TransportManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
public class TransportManager
{
private static final Logger log = Logger.getLogger(TransportManager.class);
private static final String PROPERTY_TIMEOUT = TransportManager.class.getName() + ".timeout";
private static long DEFAULT_WAIT_TIMEOUT = Long.parseLong(System.getProperty(PROPERTY_TIMEOUT,"1200000"));

class HandlerEntry
{
Expand Down Expand Up @@ -87,7 +89,7 @@ public void run()

try
{
asynchronousQueue.wait(2000);
asynchronousQueue.wait(DEFAULT_WAIT_TIMEOUT);

This comment has been minimized.

Copy link
@Elisedlund-ericsson

Elisedlund-ericsson Nov 13, 2024

Contributor

Currently this specific change seems to be causing some SSH sessions, to timeout because of inactivity.

commands that worked without issues in our enviroment previously but no longer:
sleep 120s
the session will be terminated because of inactivity i guess as theres no data flowing.
but it working fine with:
asynchronousQueue.wait(2000);
but not with the new:
asynchronousQueue.wait(DEFAULT_WAIT_TIMEOUT);

notice the /* After the queue is empty for about 2 seconds, stop this thread */
is still left and i think it was a mistake to change this specific wait() with to a different timeout.

}
catch (InterruptedException e)
{
Expand Down Expand Up @@ -685,7 +687,7 @@ public void sendMessage(byte[] msg) throws IOException

try
{
connectionSemaphore.wait();
connectionSemaphore.wait(DEFAULT_WAIT_TIMEOUT);
}
catch (InterruptedException e)
{
Expand Down

0 comments on commit 15622af

Please sign in to comment.