From 0b1fb651e94ebd2d34911f793c45aedd6b4af41d Mon Sep 17 00:00:00 2001 From: zzz Date: Thu, 18 Mar 2021 09:07:45 -0400 Subject: [PATCH] EepGet timeout fixes part 2 Have SocketTimeout interrupt thread Cancel SocketTimeout and use soTImeout after reading headers if able Cancel SocketTimeout in finally Cleanups and javadocs --- core/java/src/net/i2p/util/EepGet.java | 56 ++++++++++++------- core/java/src/net/i2p/util/EepHead.java | 8 +-- core/java/src/net/i2p/util/SSLEepGet.java | 16 ++++-- core/java/src/net/i2p/util/SocketTimeout.java | 4 +- 4 files changed, 53 insertions(+), 31 deletions(-) diff --git a/core/java/src/net/i2p/util/EepGet.java b/core/java/src/net/i2p/util/EepGet.java index 27b55ab37..e73744a3d 100644 --- a/core/java/src/net/i2p/util/EepGet.java +++ b/core/java/src/net/i2p/util/EepGet.java @@ -83,9 +83,9 @@ public class EepGet { protected boolean _notModified; protected String _contentType; protected boolean _transferFailed; - protected boolean _aborted; + protected volatile boolean _aborted; protected int _fetchHeaderTimeout; - private long _fetchEndTime; + protected int _fetchTotalTimeout; protected int _fetchInactivityTimeout; protected int _redirects; protected String _redirectLocation; @@ -476,6 +476,7 @@ public class EepGet { public void attempting(String url); } + protected class CLIStatusListener implements StatusListener { private final int _markSize; private final int _lineSize; @@ -657,8 +658,11 @@ public class EepGet { * @return success */ public boolean fetch(long fetchHeaderTimeout, long totalTimeout, long inactivityTimeout) { + // we need a SocketTimeout if we have a totalTimeout + if (totalTimeout > 0 && fetchHeaderTimeout <= 0) + fetchHeaderTimeout = totalTimeout; _fetchHeaderTimeout = (int) Math.min(fetchHeaderTimeout, Integer.MAX_VALUE); - _fetchEndTime = (totalTimeout > 0 ? System.currentTimeMillis() + totalTimeout : -1); + _fetchTotalTimeout = (int) Math.min(totalTimeout, Integer.MAX_VALUE); _fetchInactivityTimeout = (int) Math.min(inactivityTimeout, Integer.MAX_VALUE); _keepFetching = true; @@ -667,16 +671,21 @@ public class EepGet { while (_keepFetching) { SocketTimeout timeout = null; if (_fetchHeaderTimeout > 0) { + // We create the SocketTimeout with an inactivity time of the header timeout. + // After fetchHeaders(), we must call either resetTimer() or cancel() timeout = new SocketTimeout(_fetchHeaderTimeout); - final SocketTimeout stimeout = timeout; // ugly - why not use sotimeout? + final SocketTimeout stimeout = timeout; + final Thread thread = Thread.currentThread(); timeout.setTimeoutCommand(new Runnable() { public void run() { if (_log.shouldLog(Log.DEBUG)) _log.debug("timeout reached on " + _url + ": " + stimeout); _aborted = true; + thread.interrupt(); } }); - timeout.setTotalTimeoutPeriod(_fetchEndTime); + if (_fetchTotalTimeout > 0) + timeout.setTotalTimeoutPeriod(_fetchTotalTimeout); } try { for (int i = 0; i < _listeners.size(); i++) @@ -685,14 +694,10 @@ public class EepGet { if (timeout != null) timeout.resetTimer(); doFetch(timeout); - if (timeout != null) - timeout.cancel(); if (!_transferFailed) return true; break; } catch (IOException ioe) { - if (timeout != null) - timeout.cancel(); for (int i = 0; i < _listeners.size(); i++) _listeners.get(i).attemptFailed(_url, _bytesTransferred, _bytesRemaining, _currentAttempt, _numRetries, ioe); if (_log.shouldLog(Log.WARN)) @@ -702,6 +707,8 @@ public class EepGet { ioe instanceof ConnectException) // proxy or nonproxied host Connection Refused _keepFetching = false; } finally { + if (timeout != null) + timeout.cancel(); if (_out != null) { try { _out.close(); @@ -736,7 +743,8 @@ public class EepGet { } /** - * single fetch + * This reads the response to a single fetch. + * Call after sendRequest() * @param timeout may be null */ protected void doFetch(SocketTimeout timeout) throws IOException { @@ -745,20 +753,28 @@ public class EepGet { if (_aborted) throw new IOException("Timed out reading the HTTP headers"); - if (timeout != null) { - timeout.resetTimer(); - if (_fetchInactivityTimeout > 0) - timeout.setInactivityTimeout(_fetchInactivityTimeout); - else - timeout.setInactivityTimeout(INACTIVITY_TIMEOUT); - } // _proxy is null when extended by I2PSocketEepGet - if (_proxy != null && !_shouldProxy) { + if (_proxy != null) { + if (timeout != null) { + if (_fetchTotalTimeout > 0) { + timeout.resetTimer(); + } else { + // we don't need the timeout any more, we'll use soTimeout + timeout.cancel(); + timeout = null; + } + } // we only set the soTimeout before the headers if not proxied if (_fetchInactivityTimeout > 0) _proxy.setSoTimeout(_fetchInactivityTimeout); else _proxy.setSoTimeout(INACTIVITY_TIMEOUT); + } else if (timeout != null) { + timeout.resetTimer(); + if (_fetchInactivityTimeout > 0) + timeout.setInactivityTimeout(_fetchInactivityTimeout); + else + timeout.setInactivityTimeout(INACTIVITY_TIMEOUT); } if (_redirectLocation != null) { @@ -1382,7 +1398,7 @@ public class EepGet { if ("http".equals(url.getScheme())) { String host = url.getHost(); if (host == null) - throw new MalformedURLException("URL is not supported:" + _actualURL); + throw new MalformedURLException("URL is not supported: " + _actualURL); String hostlc = host.toLowerCase(Locale.US); if (hostlc.endsWith(".i2p")) throw new UnknownHostException("I2P addresses must be proxied"); @@ -1399,7 +1415,7 @@ public class EepGet { _proxy = new Socket(host, port); } } else { - throw new MalformedURLException("URL is not supported:" + _actualURL); + throw new MalformedURLException("URL is not supported: " + _actualURL); } } catch (URISyntaxException use) { IOException ioe = new MalformedURLException("Request URL is invalid"); diff --git a/core/java/src/net/i2p/util/EepHead.java b/core/java/src/net/i2p/util/EepHead.java index 692f451e5..8eca07d81 100644 --- a/core/java/src/net/i2p/util/EepHead.java +++ b/core/java/src/net/i2p/util/EepHead.java @@ -37,7 +37,7 @@ import net.i2p.I2PAppContext; */ public class EepHead extends EepGet { /** EepGet needs either a non-null file or a stream... shouldn't actually be written to... */ - static final OutputStream _dummyStream = new ByteArrayOutputStream(0); + private static final OutputStream _dummyStream = new ByteArrayOutputStream(0); public EepHead(I2PAppContext ctx, String proxyHost, int proxyPort, int numRetries, String url) { // we're using this constructor: @@ -178,7 +178,7 @@ public class EepHead extends EepGet { if (_fetchInactivityTimeout > 0) timeout.setInactivityTimeout(_fetchInactivityTimeout); else - timeout.setInactivityTimeout(60*1000); + timeout.setInactivityTimeout(INACTIVITY_TIMEOUT); } // Should we even follow redirects for HEAD? @@ -239,6 +239,8 @@ public class EepHead extends EepGet { doFetch(timeout); return; } + if (timeout != null) + timeout.cancel(); if (_log.shouldLog(Log.DEBUG)) _log.debug("Headers read completely"); @@ -250,8 +252,6 @@ public class EepHead extends EepGet { if (_aborted) throw new IOException("Timed out reading the HTTP data"); - timeout.cancel(); - if (_transferFailed) { // 404, etc - transferFailed is called after all attempts fail, by fetch() above for (int i = 0; i < _listeners.size(); i++) diff --git a/core/java/src/net/i2p/util/SSLEepGet.java b/core/java/src/net/i2p/util/SSLEepGet.java index b5771b349..236a4538a 100644 --- a/core/java/src/net/i2p/util/SSLEepGet.java +++ b/core/java/src/net/i2p/util/SSLEepGet.java @@ -586,12 +586,16 @@ public class SSLEepGet extends EepGet { if (_aborted) throw new IOException("Timed out reading the HTTP headers"); + // _proxy is the socket, even if not proxied. + // We never use the timeout inactivity timer; only socket SoTimeout. if (timeout != null) { - timeout.resetTimer(); - if (_fetchInactivityTimeout > 0) - timeout.setInactivityTimeout(_fetchInactivityTimeout); - else - timeout.setInactivityTimeout(60*1000); + if (_fetchTotalTimeout > 0) { + timeout.resetTimer(); + } else { + // we don't need the timeout any more, we'll use soTimeout + timeout.cancel(); + timeout = null; + } } if (_fetchInactivityTimeout > 0) _proxy.setSoTimeout(_fetchInactivityTimeout); @@ -823,6 +827,8 @@ public class SSLEepGet extends EepGet { _proxy.setSoTimeout(_fetchHeaderTimeout); } } + if (timeout != null) + timeout.setSocket(_proxy); SSLSocket socket = (SSLSocket) _proxy; I2PSSLSocketFactory.setProtocolsAndCiphers(socket); diff --git a/core/java/src/net/i2p/util/SocketTimeout.java b/core/java/src/net/i2p/util/SocketTimeout.java index 5beed6963..422b07f70 100644 --- a/core/java/src/net/i2p/util/SocketTimeout.java +++ b/core/java/src/net/i2p/util/SocketTimeout.java @@ -31,7 +31,7 @@ public class SocketTimeout extends SimpleTimer2.TimedEvent { private volatile Runnable _command; /** - * @param delay greater than zero + * @param delay The inactivity delay, greater than zero */ public SocketTimeout(long delay) { this(null, delay); } @@ -40,7 +40,7 @@ public class SocketTimeout extends SimpleTimer2.TimedEvent { * it will be closed when the timer expires. * * @param socket may be null - * @param delay greater than zero + * @param delay The inactivity delay, greater than zero */ public SocketTimeout(Socket socket, long delay) { super(SimpleTimer2.getInstance());