elsloo closed pull request #2335: Fix for Traffic Router file leak when TM or
TO is down
URL: https://github.com/apache/trafficcontrol/pull/2335
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/AbstractResourceWatcher.java
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/AbstractResourceWatcher.java
index 24b2008db..a9c634d00 100644
---
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/AbstractResourceWatcher.java
+++
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/AbstractResourceWatcher.java
@@ -129,13 +129,16 @@ public boolean verifyDatabase(final File dbFile) throws
IOException {
@Override
protected File downloadDatabase(final String url, final File
existingDb) {
+ if ((trafficOpsUtils.getHostname() == null) ||
trafficOpsUtils.getCdnName() == null) {
+ return null;
+ }
+ final String interpolatedUrl =
trafficOpsUtils.replaceTokens(url);
if (fetcher == null) {
- LOGGER.warn("[" + getClass().getSimpleName() + "]
Waiting for configuration to be processed, unable to download from '" + url +
"'");
+ LOGGER.warn("[" + getClass().getSimpleName() + "]
Waiting for configuration to be processed, unable to download from '" +
interpolatedUrl + "'");
return null;
}
String jsonData = null;
- final String interpolatedUrl =
trafficOpsUtils.replaceTokens(url);
try {
jsonData =
fetcher.fetchIfModifiedSince(interpolatedUrl, existingDb.lastModified());
}
diff --git
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/Fetcher.java
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/Fetcher.java
index 772c00c39..8c62cced8 100644
---
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/Fetcher.java
+++
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/Fetcher.java
@@ -72,53 +72,60 @@ public void checkServerTrusted(final X509Certificate[]
arg0, final String arg1)
}
protected HttpURLConnection getConnection(final String url, final
String data, final String requestMethod, final long lastFetchTime) throws
IOException {
- String method = GET_STR;
+ HttpURLConnection http = null;
+ try {
+ String method = GET_STR;
- if (requestMethod != null) {
- method = requestMethod;
- }
+ if (requestMethod != null) {
+ method = requestMethod;
+ }
- LOGGER.info(method + "ing: " + url + "; timeout is " + timeout);
+ LOGGER.info(method + "ing: " + url + "; timeout is " +
timeout);
- final URLConnection connection = new URL(url).openConnection();
+ final URLConnection connection = new
URL(url).openConnection();
- connection.setIfModifiedSince(lastFetchTime);
+ connection.setIfModifiedSince(lastFetchTime);
- if (timeout != 0) {
- connection.setConnectTimeout(timeout);
- connection.setReadTimeout(timeout);
- }
+ if (timeout != 0) {
+ connection.setConnectTimeout(timeout);
+ connection.setReadTimeout(timeout);
+ }
- final HttpURLConnection http = (HttpURLConnection) connection;
+ http = (HttpURLConnection) connection;
- if (connection instanceof HttpsURLConnection) {
- final HttpsURLConnection https = (HttpsURLConnection)
connection;
- https.setHostnameVerifier(new HostnameVerifier() {
- @Override
- public boolean verify(final String arg0, final
SSLSession arg1) {
- return true;
- }
- });
- }
+ if (connection instanceof HttpsURLConnection) {
+ final HttpsURLConnection https =
(HttpsURLConnection) connection;
+ https.setHostnameVerifier(new
HostnameVerifier() {
+ @Override
+ public boolean verify(final String
arg0, final SSLSession arg1) {
+ return true;
+ }
+ });
+ }
- http.setInstanceFollowRedirects(false);
- http.setRequestMethod(method);
- http.setAllowUserInteraction(true);
- http.addRequestProperty("Accept-Encoding",
GZIP_ENCODING_STRING);
+ http.setInstanceFollowRedirects(false);
+ http.setRequestMethod(method);
+ http.setAllowUserInteraction(true);
+ http.addRequestProperty("Accept-Encoding",
GZIP_ENCODING_STRING);
- for (final String key : requestProps.keySet()) {
- http.addRequestProperty(key, requestProps.get(key));
- }
+ for (final String key : requestProps.keySet()) {
+ http.addRequestProperty(key,
requestProps.get(key));
+ }
- if (method.equals(POST_STR) && data != null) {
- http.setDoOutput(true); // Triggers POST.
+ if (method.equals(POST_STR) && data != null) {
+ http.setDoOutput(true); // Triggers POST.
- try (final OutputStream output =
http.getOutputStream()) {
- output.write(data.getBytes(UTF8_STR));
+ try (final OutputStream output =
http.getOutputStream()) {
+ output.write(data.getBytes(UTF8_STR));
+ }
}
- }
- connection.connect();
+ connection.connect();
+
+ } catch (Exception e) {
+ LOGGER.error("Failed Http Request to " + http.getURL()
+ " Status " + http.getResponseCode());
+ http.disconnect();
+ }
return http;
}
@@ -133,44 +140,50 @@ public String fetch(final String url) throws IOException {
private String fetchIfModifiedSince(final String url, final String
data, final String method, final long lastFetchTime) throws IOException {
final OutputStream out = null;
+ String ifModifiedSince = null;
try {
final HttpURLConnection connection = getConnection(url,
data, method, lastFetchTime);
+ if (connection != null) {
+ if (connection.getResponseCode() ==
HttpURLConnection.HTTP_NOT_MODIFIED) {
+ return null;
+ }
- if (connection.getResponseCode() ==
HttpURLConnection.HTTP_NOT_MODIFIED) {
- return null;
- }
+ if (connection.getResponseCode() > 399) {
+ LOGGER.warn("Failed Http Request to " +
url + " Status " + connection.getResponseCode());
+ return null;
+ }
- if (connection.getResponseCode() > 399) {
- LOGGER.warn("Failed Http Request to " + url + "
Status " + connection.getResponseCode());
- return null;
+ final StringBuilder sb = new StringBuilder();
+ createStringBuilderFromResponse(sb, connection);
+ ifModifiedSince = sb.toString();
}
- final StringBuilder sb = new StringBuilder();
- createStringBuilderFromResponse(sb, connection);
-
- return sb.toString();
} finally {
IOUtils.closeQuietly(out);
}
+ return ifModifiedSince;
}
public int getIfModifiedSince(final String url, final long
lastFetchTime, final StringBuilder stringBuilder) throws IOException {
final OutputStream out = null;
+ int status = 0;
try {
final HttpURLConnection connection = getConnection(url,
null, "GET", lastFetchTime);
- final int status = connection.getResponseCode();
+ if (connection != null) {
+ status = connection.getResponseCode();
- if (status == HttpURLConnection.HTTP_NOT_MODIFIED) {
- return status;
- }
+ if (status ==
HttpURLConnection.HTTP_NOT_MODIFIED) {
+ return status;
+ }
- if (connection.getResponseCode() > 399) {
- LOGGER.warn("Failed Http Request to " + url + "
Status " + connection.getResponseCode());
- return status;
- }
+ if (connection.getResponseCode() > 399) {
+ LOGGER.warn("Failed Http Request to " +
url + " Status " + connection.getResponseCode());
+ return status;
+ }
- createStringBuilderFromResponse(stringBuilder,
connection);
+ createStringBuilderFromResponse(stringBuilder,
connection);
+ }
return status;
} finally {
IOUtils.closeQuietly(out);
diff --git
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/PeriodicResourceUpdater.java
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/PeriodicResourceUpdater.java
index 3072b6564..5415bfbbf 100644
---
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/PeriodicResourceUpdater.java
+++
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/PeriodicResourceUpdater.java
@@ -52,11 +52,7 @@
public class PeriodicResourceUpdater {
private static final Logger LOGGER =
Logger.getLogger(PeriodicResourceUpdater.class);
- private static final AsyncHttpClient asyncHttpClient = new
AsyncHttpClient(
- new AsyncHttpClientConfig.Builder()
- .setConnectionTimeoutInMs(10000)
- .build());
-
+ private AsyncHttpClient asyncHttpClient;
protected String databaseLocation;
protected final ResourceUrl urls;
protected ScheduledExecutorService executorService =
Executors.newSingleThreadScheduledExecutor();
@@ -105,6 +101,7 @@ public void run() {
final private boolean pauseTilLoaded;
public void init() {
+ asyncHttpClient = newAsyncClient();
putCurrent();
LOGGER.info("Starting schedule with interval:
"+getPollingInterval() + " : "+TimeUnit.MILLISECONDS);
scheduledService =
executorService.scheduleWithFixedDelay(updater, 0, getPollingInterval(),
TimeUnit.MILLISECONDS);
@@ -121,6 +118,13 @@ public void init() {
}
}
+ private AsyncHttpClient newAsyncClient() {
+ return new AsyncHttpClient(
+ new AsyncHttpClientConfig.Builder()
+ .setConnectionTimeoutInMs(10000)
+ .build());
+ }
+
private synchronized void putCurrent() {
final File existingDB = new File(databaseLocation);
if(existingDB.exists()) {
@@ -139,7 +143,9 @@ public synchronized boolean updateDatabase() {
final Request request =
getRequest(urls.nextUrl());
if (request != null) {
request.getHeaders().add("Accept-Encoding", GZIP_ENCODING_STRING);
- asyncHttpClient.executeRequest(request,
new UpdateHandler(request)); // AsyncHandlers are NOT thread safe; one instance
per request
+ if ((asyncHttpClient!=null) &&
(!asyncHttpClient.isClosed())) {
+
asyncHttpClient.executeRequest(request, new UpdateHandler(request)); //
AsyncHandlers are NOT thread safe; one instance per request
+ }
return true;
}
} else {
@@ -284,6 +290,12 @@ public Integer onCompleted(final Response response) throws
IOException {
@Override
public void onThrowable(final Throwable t){
LOGGER.warn("Failed request " + request.getUrl() + ": "
+ t, t);
+ if (asyncHttpClient!=null) {
+ while (!asyncHttpClient.isClosed()) {
+ asyncHttpClient.close();
+ }
+ }
+ asyncHttpClient = newAsyncClient();
}
};
diff --git
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/ProtectedFetcher.java
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/ProtectedFetcher.java
index 10da44f14..0edb4292d 100644
---
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/ProtectedFetcher.java
+++
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/ProtectedFetcher.java
@@ -40,10 +40,9 @@ protected HttpURLConnection getConnection(final String url,
final String data, f
}
private HttpURLConnection extractCookie(final HttpURLConnection http)
throws IOException {
- if (http.getHeaderField("Set-Cookie") != null) {
+ if ((http != null) && (http.getHeaderField("Set-Cookie") !=
null)) {
setCookie(HttpCookie.parse(http.getHeaderField("Set-Cookie")).get(0));
}
-
return http;
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
