From fccff721935973648b96bd2af6f4d0a2d9b56bae Mon Sep 17 00:00:00 2001 From: slewis Date: Wed, 19 Mar 2014 12:01:34 -0700 Subject: Proposed fix for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=430704 for the httpclient3 provider Change-Id: I0000000000000000000000000000000000000000 --- .../META-INF/MANIFEST.MF | 2 +- .../httpclient/ConnectingSocketMonitor.java | 47 +++++++++++++++++++++- .../httpclient/HttpClientFileSystemBrowser.java | 12 +----- .../httpclient/HttpClientRetrieveFileTransfer.java | 12 +----- 4 files changed, 48 insertions(+), 25 deletions(-) diff --git a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/META-INF/MANIFEST.MF b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/META-INF/MANIFEST.MF index 7120b3e08..f2b2c872e 100644 --- a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/META-INF/MANIFEST.MF +++ b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %plugin.name Bundle-SymbolicName: org.eclipse.ecf.provider.filetransfer.httpclient;singleton:=true -Bundle-Version: 4.0.300.qualifier +Bundle-Version: 4.0.400.qualifier Bundle-Localization: plugin Bundle-Activator: org.eclipse.ecf.internal.provider.filetransfer.httpclient.Activator Require-Bundle: org.eclipse.equinox.common, diff --git a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/internal/provider/filetransfer/httpclient/ConnectingSocketMonitor.java b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/internal/provider/filetransfer/httpclient/ConnectingSocketMonitor.java index 39d8f5aca..407e17651 100644 --- a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/internal/provider/filetransfer/httpclient/ConnectingSocketMonitor.java +++ b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/internal/provider/filetransfer/httpclient/ConnectingSocketMonitor.java @@ -11,8 +11,19 @@ package org.eclipse.ecf.internal.provider.filetransfer.httpclient; -import java.util.*; -import org.eclipse.ecf.filetransfer.events.socket.*; +import java.io.IOException; +import java.net.Socket; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import org.eclipse.ecf.core.util.Trace; +import org.eclipse.ecf.filetransfer.events.socket.ISocketClosedEvent; +import org.eclipse.ecf.filetransfer.events.socket.ISocketConnectedEvent; +import org.eclipse.ecf.filetransfer.events.socket.ISocketCreatedEvent; +import org.eclipse.ecf.filetransfer.events.socket.ISocketEvent; +import org.eclipse.ecf.filetransfer.events.socket.ISocketListener; public class ConnectingSocketMonitor implements ISocketListener { @@ -26,6 +37,16 @@ public class ConnectingSocketMonitor implements ISocketListener { connectingSockets = Collections.synchronizedMap(new HashMap()); } + /** + * Callers of this method should not iterate through the returned + * Collection, as a CME is possible...as reported by bug + * https://bugs.eclipse.org/bugs/show_bug.cgi?id=430704 + * Rather than call this method and iterate through the Collection, + * to close the connecting sockets call closeConnectingSockets + * instead. + * @return Collection the existing collection of underlying connecting + * Socket instances + */ public Collection getConnectingSockets() { return Collections.unmodifiableCollection(connectingSockets.keySet()); } @@ -34,6 +55,28 @@ public class ConnectingSocketMonitor implements ISocketListener { connectingSockets.clear(); } + /** + * Method added to synchronize access to underlying keySet + * to prevent CME as reported in bug + * https://bugs.eclipse.org/bugs/show_bug.cgi?id=430704 + */ + public void closeSockets() { + // synchronize on the connectingSockets map + // so all changes caused by handleSocketEvent + // are prevented via synchronized Map + synchronized (connectingSockets) { + for (Iterator iterator = connectingSockets.keySet().iterator(); iterator.hasNext();) { + Socket socket = (Socket) iterator.next(); + try { + Trace.trace(Activator.PLUGIN_ID, "Call socket.close() for socket=" + socket.toString()); //$NON-NLS-1$ + socket.close(); + } catch (IOException e) { + Trace.catching(Activator.PLUGIN_ID, DebugOptions.EXCEPTIONS_CATCHING, this.getClass(), "cancel", e); //$NON-NLS-1$ + } + } + } + } + public void handleSocketEvent(ISocketEvent event) { if (event instanceof ISocketCreatedEvent) { connectingSockets.put(event.getFactorySocket(), event); diff --git a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/provider/filetransfer/httpclient/HttpClientFileSystemBrowser.java b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/provider/filetransfer/httpclient/HttpClientFileSystemBrowser.java index daa428875..330cff684 100644 --- a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/provider/filetransfer/httpclient/HttpClientFileSystemBrowser.java +++ b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/provider/filetransfer/httpclient/HttpClientFileSystemBrowser.java @@ -15,9 +15,7 @@ package org.eclipse.ecf.provider.filetransfer.httpclient; import java.io.IOException; import java.net.HttpURLConnection; -import java.net.Socket; import java.net.URL; -import java.util.Iterator; import java.util.Map; import org.apache.commons.httpclient.Credentials; import org.apache.commons.httpclient.Header; @@ -144,15 +142,7 @@ public class HttpClientFileSystemBrowser extends AbstractFileSystemBrowser { } } if (connectingSockets != null) { - // this should unblock socket connect calls, if any - for (Iterator iterator = connectingSockets.getConnectingSockets().iterator(); iterator.hasNext();) { - Socket socket = (Socket) iterator.next(); - try { - socket.close(); - } catch (IOException e) { - Trace.catching(Activator.PLUGIN_ID, DebugOptions.EXCEPTIONS_CATCHING, this.getClass(), "cancel", e); //$NON-NLS-1$ - } - } + connectingSockets.closeSockets(); } } diff --git a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/provider/filetransfer/httpclient/HttpClientRetrieveFileTransfer.java b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/provider/filetransfer/httpclient/HttpClientRetrieveFileTransfer.java index a58b6cb50..40a514e8e 100644 --- a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/provider/filetransfer/httpclient/HttpClientRetrieveFileTransfer.java +++ b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient/src/org/eclipse/ecf/provider/filetransfer/httpclient/HttpClientRetrieveFileTransfer.java @@ -17,7 +17,6 @@ import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.net.HttpURLConnection; -import java.net.Socket; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; @@ -370,16 +369,7 @@ public class HttpClientRetrieveFileTransfer extends AbstractRetrieveFileTransfer } } if (connectingSockets != null) { - // this should unblock socket connect calls, if any - for (Iterator iterator = connectingSockets.getConnectingSockets().iterator(); iterator.hasNext();) { - Socket socket = (Socket) iterator.next(); - try { - Trace.trace(Activator.PLUGIN_ID, "Call socket.close() for socket=" + socket.toString()); //$NON-NLS-1$ - socket.close(); - } catch (IOException e) { - Trace.catching(Activator.PLUGIN_ID, DebugOptions.EXCEPTIONS_CATCHING, this.getClass(), "cancel", e); //$NON-NLS-1$ - } - } + connectingSockets.closeSockets(); } hardClose(); if (fireDoneEvent) { -- cgit v1.2.3