commit 8c14bac87e3623bf7d8c1e04f9edc84c5a2a64c0 Author: Karsten Loesing <karsten.loesing@gmx.net> Date: Mon Jan 23 19:40:26 2012 +0100 Use timeouts in URLConnection instead of own timeout thread. --- .../torproject/descriptor/DescriptorRequest.java | 10 +++- .../descriptor/RelayDescriptorDownloader.java | 12 ++++- .../descriptor/example/ConsensusHealthChecker.java | 10 +++-- .../example/MetricsRelayDescriptorAggregator.java | 9 ++-- .../descriptor/impl/DescriptorRequestImpl.java | 11 ++++- .../descriptor/impl/DirectoryDownloader.java | 44 ++++--------------- .../descriptor/impl/DownloadCoordinatorImpl.java | 15 ++++-- .../impl/RelayDescriptorDownloaderImpl.java | 30 ++++++++++---- 8 files changed, 77 insertions(+), 64 deletions(-) diff --git a/src/org/torproject/descriptor/DescriptorRequest.java b/src/org/torproject/descriptor/DescriptorRequest.java index f68332f..422d133 100644 --- a/src/org/torproject/descriptor/DescriptorRequest.java +++ b/src/org/torproject/descriptor/DescriptorRequest.java @@ -29,12 +29,16 @@ public interface DescriptorRequest { /* Return the time in millis when this request ended. */ public long getRequestEnd(); - /* Return whether this request ended, because the request timeout + /* Return whether this request ended, because the connect timeout has * expired. */ - public boolean requestTimeoutHasExpired(); + public boolean connectTimeoutHasExpired(); + + /* Return whether this request ended, because the read timeout has + * expired. */ + public boolean readTimeoutHasExpired(); /* Return whether this request ended, because the global timeout for all - * requests expired. */ + * requests has expired. */ public boolean globalTimeoutHasExpired(); /* Return the descriptors contained in the reply. */ diff --git a/src/org/torproject/descriptor/RelayDescriptorDownloader.java b/src/org/torproject/descriptor/RelayDescriptorDownloader.java index 28d6ea1..754c644 100644 --- a/src/org/torproject/descriptor/RelayDescriptorDownloader.java +++ b/src/org/torproject/descriptor/RelayDescriptorDownloader.java @@ -74,11 +74,17 @@ public interface RelayDescriptorDownloader { * descriptors. */ public void setExcludeExtraInfoDescriptors(Set<String> identifiers); - /* Define a request timeout for a single request. If a timeout expires, + /* Define a connect timeout for a single request. If a timeout expires, * no further requests will be sent to the directory authority or - * mirror. Setting this value to 0 disables the request timeout. + * mirror. Setting this value to 0 disables the connect timeout. * Default value is 1 minute (60 * 1000). */ - public void setRequestTimeout(long requestTimeoutMillis); + public void setConnectTimeout(long connectTimeoutMillis); + + /* Define a read timeout for a single request. If a timeout expires, + * no further requests will be sent to the directory authority or + * mirror. Setting this value to 0 disables the read timeout. + * Default value is 1 minute (60 * 1000). */ + public void setReadTimeout(long readTimeoutMillis); /* Define a global timeout for all requests. Once this timeout expires, * all running requests are aborted and no further requests are made. diff --git a/src/org/torproject/descriptor/example/ConsensusHealthChecker.java b/src/org/torproject/descriptor/example/ConsensusHealthChecker.java index 178f164..b8cc12b 100644 --- a/src/org/torproject/descriptor/example/ConsensusHealthChecker.java +++ b/src/org/torproject/descriptor/example/ConsensusHealthChecker.java @@ -38,9 +38,10 @@ public class ConsensusHealthChecker { downloader.setIncludeCurrentConsensusFromAllDirectoryAuthorities(); downloader.setIncludeCurrentReferencedVotes(); - /* Set a request timeout of 1 minute and a global timeout of 10 - * minutes to avoid being blocked forever by a slow download. */ - downloader.setRequestTimeout(60L * 1000L); + /* Set connect and read timeouts of 1 minute each and a global timeout + * of 10 minutes to avoid being blocked forever by a slow download. */ + downloader.setConnectTimeout(60L * 1000L); + downloader.setReadTimeout(60L * 1000L); downloader.setGlobalTimeout(10L * 60L * 1000L); /* Run the previously configured downloads and iterate over the @@ -60,7 +61,8 @@ public class ConsensusHealthChecker { + "more consensuses and/or votes and cannot make a good " + "statement about the consensus health. Exiting."); return; - } else if (request.requestTimeoutHasExpired()) { + } else if (request.connectTimeoutHasExpired() || + request.readTimeoutHasExpired()) { System.out.println("The request to directory authority " + request.getDirectoryNickname() + " to download the " + "descriptor(s) at " + request.getRequestUrl() + " has " diff --git a/src/org/torproject/descriptor/example/MetricsRelayDescriptorAggregator.java b/src/org/torproject/descriptor/example/MetricsRelayDescriptorAggregator.java index 0d9da8d..bfaaa22 100644 --- a/src/org/torproject/descriptor/example/MetricsRelayDescriptorAggregator.java +++ b/src/org/torproject/descriptor/example/MetricsRelayDescriptorAggregator.java @@ -99,10 +99,11 @@ public class MetricsRelayDescriptorAggregator { downloader.setExcludeExtraInfoDescriptors( knownExtraInfoDescriptorIdentifiers.keySet()); - /* Set a request timeout of 2 minutes and a global timeout of 1 hour - * to avoid being blocked forever by a slow download, but also to - * avoid giving up too quickly. */ - downloader.setRequestTimeout(2L * 60L * 1000L); + /* Set connect and read timeouts of 2 minutes each and a global + * timeout of 1 hour to avoid being blocked forever by a slow + * download, but also to avoid giving up too quickly. */ + downloader.setConnectTimeout(2L * 60L * 1000L); + downloader.setReadTimeout(2L * 60L * 1000L); downloader.setGlobalTimeout(60L * 60L * 1000L); /* Download descriptors and process them. */ diff --git a/src/org/torproject/descriptor/impl/DescriptorRequestImpl.java b/src/org/torproject/descriptor/impl/DescriptorRequestImpl.java index 6c44a79..e5c2ff5 100644 --- a/src/org/torproject/descriptor/impl/DescriptorRequestImpl.java +++ b/src/org/torproject/descriptor/impl/DescriptorRequestImpl.java @@ -70,9 +70,14 @@ public class DescriptorRequestImpl implements DescriptorRequest { return this.requestEnd; } - private boolean requestTimeoutHasExpired; - public boolean requestTimeoutHasExpired() { - return this.requestTimeoutHasExpired; + private boolean connectTimeoutHasExpired; + public boolean connectTimeoutHasExpired() { + return this.connectTimeoutHasExpired; + } + + private boolean readTimeoutHasExpired; + public boolean readTimeoutHasExpired() { + return this.readTimeoutHasExpired; } private boolean globalTimeoutHasExpired; diff --git a/src/org/torproject/descriptor/impl/DirectoryDownloader.java b/src/org/torproject/descriptor/impl/DirectoryDownloader.java index 7b4311c..7e9d19c 100644 --- a/src/org/torproject/descriptor/impl/DirectoryDownloader.java +++ b/src/org/torproject/descriptor/impl/DirectoryDownloader.java @@ -27,9 +27,14 @@ public class DirectoryDownloader implements Runnable { this.downloadCoordinator = downloadCoordinator; } - private long requestTimeout; - protected void setRequestTimeout(long requestTimeout) { - this.requestTimeout = requestTimeout; + private long connectTimeout; + protected void setConnectTimeout(long connectTimeout) { + this.connectTimeout = connectTimeout; + } + + private long readTimeout; + protected void setReadTimeout(long readTimeout) { + this.readTimeout = readTimeout; } public void run() { @@ -41,13 +46,12 @@ public class DirectoryDownloader implements Runnable { String url = "http://" + this.ipPort + request.getRequestedResource(); request.setRequestStart(System.currentTimeMillis()); - Thread timeoutThread = new Thread(new RequestTimeout( - this.requestTimeout)); - timeoutThread.start(); try { URL u = new URL(url); HttpURLConnection huc = (HttpURLConnection) u.openConnection(); + huc.setConnectTimeout((int) this.connectTimeout); + huc.setReadTimeout((int) this.readTimeout); huc.setRequestMethod("GET"); huc.connect(); int responseCode = huc.getResponseCode(); @@ -74,39 +78,11 @@ public class DirectoryDownloader implements Runnable { * problems, e.g., refused connections. */ keepRunning = false; } - /* TODO How do we find out if we were interrupted, and by who? - * Set the request or global timeout flag in the response. */ - timeoutThread.interrupt(); this.downloadCoordinator.deliverResponse(request); } else { keepRunning = false; } } while (keepRunning); } - - /* Interrupt a download request if it takes longer than a given time. */ - /* TODO Also look at URLConnection.setConnectTimeout() and - * URLConnection.setReadTimeout() instead of implementing this - * ourselves. */ - private static class RequestTimeout implements Runnable { - private long timeoutMillis; - private Thread downloaderThread; - private RequestTimeout(long timeoutMillis) { - this.downloaderThread = Thread.currentThread(); - this.timeoutMillis = timeoutMillis; - } - public void run() { - long started = System.currentTimeMillis(), sleep; - while ((sleep = started + this.timeoutMillis - - System.currentTimeMillis()) > 0L) { - try { - Thread.sleep(sleep); - } catch (InterruptedException e) { - return; - } - } - this.downloaderThread.interrupt(); - } - } } diff --git a/src/org/torproject/descriptor/impl/DownloadCoordinatorImpl.java b/src/org/torproject/descriptor/impl/DownloadCoordinatorImpl.java index e91fd8b..03e7997 100644 --- a/src/org/torproject/descriptor/impl/DownloadCoordinatorImpl.java +++ b/src/org/torproject/descriptor/impl/DownloadCoordinatorImpl.java @@ -27,7 +27,8 @@ public class DownloadCoordinatorImpl implements DownloadCoordinator { private SortedMap<String, DirectoryDownloader> directoryMirrors; private boolean downloadConsensusFromAllAuthorities; private boolean includeCurrentReferencedVotes; - private long requestTimeoutMillis; + private long connectTimeoutMillis; + private long readTimeoutMillis; private long globalTimeoutMillis; protected DownloadCoordinatorImpl( @@ -36,7 +37,8 @@ public class DownloadCoordinatorImpl implements DownloadCoordinator { boolean downloadConsensus, boolean downloadConsensusFromAllAuthorities, Set<String> downloadVotes, boolean includeCurrentReferencedVotes, - long requestTimeoutMillis, long globalTimeoutMillis) { + long connectTimeoutMillis, long readTimeoutMillis, + long globalTimeoutMillis) { this.directoryAuthorities = directoryAuthorities; this.directoryMirrors = directoryMirrors; this.missingConsensus = downloadConsensus; @@ -44,7 +46,8 @@ public class DownloadCoordinatorImpl implements DownloadCoordinator { downloadConsensusFromAllAuthorities; this.missingVotes = downloadVotes; this.includeCurrentReferencedVotes = includeCurrentReferencedVotes; - this.requestTimeoutMillis = requestTimeoutMillis; + this.connectTimeoutMillis = connectTimeoutMillis; + this.readTimeoutMillis = readTimeoutMillis; this.globalTimeoutMillis = globalTimeoutMillis; if (this.directoryMirrors.isEmpty() && this.directoryAuthorities.isEmpty()) { @@ -59,13 +62,15 @@ public class DownloadCoordinatorImpl implements DownloadCoordinator { for (DirectoryDownloader directoryMirror : this.directoryMirrors.values()) { directoryMirror.setDownloadCoordinator(this); - directoryMirror.setRequestTimeout(this.requestTimeoutMillis); + directoryMirror.setConnectTimeout(this.connectTimeoutMillis); + directoryMirror.setReadTimeout(this.readTimeoutMillis); new Thread(directoryMirror).start(); } for (DirectoryDownloader directoryAuthority : this.directoryAuthorities.values()) { directoryAuthority.setDownloadCoordinator(this); - directoryAuthority.setRequestTimeout(this.requestTimeoutMillis); + directoryAuthority.setConnectTimeout(this.connectTimeoutMillis); + directoryAuthority.setReadTimeout(this.readTimeoutMillis); new Thread(directoryAuthority).start(); } } diff --git a/src/org/torproject/descriptor/impl/RelayDescriptorDownloaderImpl.java b/src/org/torproject/descriptor/impl/RelayDescriptorDownloaderImpl.java index 7cdcc0c..0888d64 100644 --- a/src/org/torproject/descriptor/impl/RelayDescriptorDownloaderImpl.java +++ b/src/org/torproject/descriptor/impl/RelayDescriptorDownloaderImpl.java @@ -193,18 +193,32 @@ public class RelayDescriptorDownloaderImpl + "descriptors is not implemented yet."); } - private long requestTimeoutMillis = 60L * 1000L; - public void setRequestTimeout(long requestTimeoutMillis) { + private long readTimeoutMillis = 60L * 1000L; + public void setReadTimeout(long readTimeoutMillis) { if (this.hasStartedDownloading) { throw new IllegalStateException("Reconfiguration is not permitted " + "after starting to download."); } - if (requestTimeoutMillis < 0L) { - throw new IllegalArgumentException("Request timeout value " - + String.valueOf(requestTimeoutMillis) + " may not be " + if (readTimeoutMillis < 0L) { + throw new IllegalArgumentException("Read timeout value " + + String.valueOf(readTimeoutMillis) + " may not be " + "negative."); } - this.requestTimeoutMillis = requestTimeoutMillis; + this.readTimeoutMillis = readTimeoutMillis; + } + + private long connectTimeoutMillis = 60L * 1000L; + public void setConnectTimeout(long connectTimeoutMillis) { + if (this.hasStartedDownloading) { + throw new IllegalStateException("Reconfiguration is not permitted " + + "after starting to download."); + } + if (connectTimeoutMillis < 0L) { + throw new IllegalArgumentException("Connect timeout value " + + String.valueOf(connectTimeoutMillis) + " may not be " + + "negative."); + } + this.connectTimeoutMillis = connectTimeoutMillis; } private long globalTimeoutMillis = 60L * 60L * 1000L; @@ -231,8 +245,8 @@ public class RelayDescriptorDownloaderImpl new DownloadCoordinatorImpl(this.directoryAuthorities, this.directoryMirrors, this.downloadConsensus, this.downloadConsensusFromAllAuthorities, this.downloadVotes, - this.includeCurrentReferencedVotes, this.requestTimeoutMillis, - this.globalTimeoutMillis); + this.includeCurrentReferencedVotes, this.connectTimeoutMillis, + this.readTimeoutMillis, this.globalTimeoutMillis); Iterator<DescriptorRequest> descriptorQueue = downloadCoordinator. getDescriptorQueue(); return descriptorQueue;