SMACK-458 Managers should be kept on disconnects

Smack's Managers should not remove itself when the connection is closed
or should re-add themselves if the connection get reconnected.

This should also fix some NPE's.

We are currently going with two different designs of Manager: 1. The one
with WeakReferences/WeakHashMaps (SDM, EntityCapsManager) and 2. the one
where the managers remove their listeners on connectionClosed() *and*
connectionClosedOnError(), and later add their listeners on
reconnectionSuccessful(). The first design has the Connection instance
only weak referenced. The other design does reference Connection
strongly (e.g. the 'managers' map in IBBManager/S5BManager), but removes
this references when connectionClosed(onError)() is called.

git-svn-id: http://svn.igniterealtime.org/svn/repos/smack/branches/smack_3_3_2@13788 b35dd754-fafc-0310-a699-88a17e54d16e

source/org/jivesoftware/smack/PrivacyListManager.java

@@ -25,6 +25,7 @@ import org.jivesoftware.smack.packet.Packet;
 import org.jivesoftware.smack.packet.Privacy;
 import org.jivesoftware.smack.packet.PrivacyItem;
 
+import java.lang.ref.WeakReference;
 import java.util.*;
 
 /**
@@ -43,9 +44,10 @@ import java.util.*;
 public class PrivacyListManager {
 
     // Keep the list of instances of this class.
-	private static Map<Connection, PrivacyListManager> instances = new Hashtable<Connection, PrivacyListManager>();
+    private static Map<Connection, PrivacyListManager> instances = Collections
+            .synchronizedMap(new WeakHashMap<Connection, PrivacyListManager>());
 
-	private Connection connection;
+	private WeakReference<Connection> connection;
 	private final List<PrivacyListListener> listeners = new ArrayList<PrivacyListListener>();
 	PacketFilter packetFilter = new AndFilter(new IQTypeFilter(IQ.Type.SET),
     		new PacketExtensionFilter("query", "jabber:iq:privacy"));
@@ -67,49 +69,10 @@ public class PrivacyListManager {
      *
      * @param connection the XMPP connection.
      */
-	private PrivacyListManager(Connection connection) {
+	private PrivacyListManager(final Connection connection) {
-        this.connection = connection;
+        this.connection = new WeakReference<Connection>(connection);
-        this.init();
-    }
-
-	/** Answer the connection userJID that owns the privacy.
-	 * @return the userJID that owns the privacy
-	 */
-	private String getUser() {
-		return connection.getUser();
-	}
-
-    /**
-     * Initializes the packet listeners of the connection that will notify for any set privacy 
-     * package. 
-     */
-    private void init() {
         // Register the new instance and associate it with the connection 
         instances.put(connection, this);
-        // Add a listener to the connection that removes the registered instance when
-        // the connection is closed
-        connection.addConnectionListener(new ConnectionListener() {
-            public void connectionClosed() {
-                // Unregister this instance since the connection has been closed
-                instances.remove(connection);
-            }
-
-            public void connectionClosedOnError(Exception e) {
-                // ignore
-            }
-
-            public void reconnectionFailed(Exception e) {
-                // ignore
-            }
-
-            public void reconnectingIn(int seconds) {
-                // ignore
-            }
-
-            public void reconnectionSuccessful() {
-                // ignore
-            }
-        });
 
         connection.addPacketListener(new PacketListener() {
             public void processPacket(Packet packet) {
@@ -151,8 +114,14 @@ public class PrivacyListManager {
                 // Send create & join packet.
                 connection.sendPacket(iq);
             }
-        }, packetFilter);
+        }, packetFilter);    }
-    }
+
+	/** Answer the connection userJID that owns the privacy.
+	 * @return the userJID that owns the privacy
+	 */
+	private String getUser() {
+		return connection.get().getUser();
+	}
 
     /**
      * Returns the PrivacyListManager instance associated with a given Connection.
@@ -174,6 +143,8 @@ public class PrivacyListManager {
 	 * @exception XMPPException if the request or the answer failed, it raises an exception.
 	 */ 
 	private Privacy getRequest(Privacy requestPrivacy) throws XMPPException {
+        Connection connection = PrivacyListManager.this.connection.get();
+        if (connection == null) throw new XMPPException("Connection instance already gc'ed");
 		// The request is a get iq type
 		requestPrivacy.setType(Privacy.Type.GET);
 		requestPrivacy.setFrom(this.getUser());
@@ -212,7 +183,8 @@ public class PrivacyListManager {
 	 * @exception XMPPException if the request or the answer failed, it raises an exception.
 	 */ 
 	private Packet setRequest(Privacy requestPrivacy) throws XMPPException {
-		
+        Connection connection = PrivacyListManager.this.connection.get();
+        if (connection == null) throw new XMPPException("Connection instance already gc'ed");
 		// The request is a get iq type
 		requestPrivacy.setType(Privacy.Type.SET);
 		requestPrivacy.setFrom(this.getUser());

source/org/jivesoftware/smackx/ServiceDiscoveryManager.java

@@ -34,6 +34,7 @@ import org.jivesoftware.smackx.packet.DiscoverInfo.Identity;
 import org.jivesoftware.smackx.packet.DiscoverItems;
 import org.jivesoftware.smackx.packet.DataForm;
 
+import java.lang.ref.WeakReference;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -59,9 +60,9 @@ public class ServiceDiscoveryManager {
     private EntityCapsManager capsManager;
 
     private static Map<Connection, ServiceDiscoveryManager> instances =
-            new ConcurrentHashMap<Connection, ServiceDiscoveryManager>();
+            Collections.synchronizedMap(new WeakHashMap<Connection, ServiceDiscoveryManager>());
 
-    private Connection connection;
+    private WeakReference<Connection> connection;
     private final Set<String> features = new HashSet<String>();
     private DataForm extendedInfo = null;
     private Map<String, NodeInformationProvider> nodeInformationProviders =
@@ -82,12 +83,100 @@ public class ServiceDiscoveryManager {
      * service manager will respond to any service discovery request that the connection may
      * receive. 
      * 
+     * @deprecated  use {@link #getInstanceFor(connection)} instead
      * @param connection the connection to which a ServiceDiscoveryManager is going to be created.
      */
+    @Deprecated
     public ServiceDiscoveryManager(Connection connection) {
-        this.connection = connection;
+        this.connection = new WeakReference<Connection>(connection);
+        // Register the new instance and associate it with the connection 
+        instances.put(connection, this);
 
-        init();
+        addFeature(DiscoverInfo.NAMESPACE);
+        addFeature(DiscoverItems.NAMESPACE);
+
+        // Listen for disco#items requests and answer with an empty result        
+        PacketFilter packetFilter = new PacketTypeFilter(DiscoverItems.class);
+        PacketListener packetListener = new PacketListener() {
+            public void processPacket(Packet packet) {
+                Connection connection = ServiceDiscoveryManager.this.connection.get();
+                if (connection == null) return;
+                DiscoverItems discoverItems = (DiscoverItems) packet;
+                // Send back the items defined in the client if the request is of type GET
+                if (discoverItems != null && discoverItems.getType() == IQ.Type.GET) {
+                    DiscoverItems response = new DiscoverItems();
+                    response.setType(IQ.Type.RESULT);
+                    response.setTo(discoverItems.getFrom());
+                    response.setPacketID(discoverItems.getPacketID());
+                    response.setNode(discoverItems.getNode());
+
+                    // Add the defined items related to the requested node. Look for 
+                    // the NodeInformationProvider associated with the requested node.  
+                    NodeInformationProvider nodeInformationProvider =
+                            getNodeInformationProvider(discoverItems.getNode());
+                    if (nodeInformationProvider != null) {
+                        // Specified node was found, add node items
+                        response.addItems(nodeInformationProvider.getNodeItems());
+                        // Add packet extensions
+                        response.addExtensions(nodeInformationProvider.getNodePacketExtensions());
+                    } else if(discoverItems.getNode() != null) {
+                        // Return <item-not-found/> error since client doesn't contain
+                        // the specified node
+                        response.setType(IQ.Type.ERROR);
+                        response.setError(new XMPPError(XMPPError.Condition.item_not_found));
+                    }
+                    connection.sendPacket(response);
+                }
+            }
+        };
+        connection.addPacketListener(packetListener, packetFilter);
+
+        // Listen for disco#info requests and answer the client's supported features 
+        // To add a new feature as supported use the #addFeature message        
+        packetFilter = new PacketTypeFilter(DiscoverInfo.class);
+        packetListener = new PacketListener() {
+            public void processPacket(Packet packet) {
+                Connection connection = ServiceDiscoveryManager.this.connection.get();
+                if (connection == null) return;
+                DiscoverInfo discoverInfo = (DiscoverInfo) packet;
+                // Answer the client's supported features if the request is of the GET type
+                if (discoverInfo != null && discoverInfo.getType() == IQ.Type.GET) {
+                    DiscoverInfo response = new DiscoverInfo();
+                    response.setType(IQ.Type.RESULT);
+                    response.setTo(discoverInfo.getFrom());
+                    response.setPacketID(discoverInfo.getPacketID());
+                    response.setNode(discoverInfo.getNode());
+                    // Add the client's identity and features only if "node" is null
+                    // and if the request was not send to a node. If Entity Caps are
+                    // enabled the client's identity and features are may also added
+                    // if the right node is chosen
+                    if (discoverInfo.getNode() == null) {
+                        addDiscoverInfoTo(response);
+                    }
+                    else {
+                        // Disco#info was sent to a node. Check if we have information of the
+                        // specified node
+                        NodeInformationProvider nodeInformationProvider =
+                                getNodeInformationProvider(discoverInfo.getNode());
+                        if (nodeInformationProvider != null) {
+                            // Node was found. Add node features
+                            response.addFeatures(nodeInformationProvider.getNodeFeatures());
+                            // Add node identities
+                            response.addIdentities(nodeInformationProvider.getNodeIdentities());
+                            // Add packet extensions
+                            response.addExtensions(nodeInformationProvider.getNodePacketExtensions());
+                        }
+                        else {
+                            // Return <item-not-found/> error since specified node was not found
+                            response.setType(IQ.Type.ERROR);
+                            response.setError(new XMPPError(XMPPError.Condition.item_not_found));
+                        }
+                    }
+                    connection.sendPacket(response);
+                }
+            }
+        };
+        connection.addPacketListener(packetListener, packetFilter);
     }
 
     /**
@@ -96,8 +185,12 @@ public class ServiceDiscoveryManager {
      * @param connection the connection used to look for the proper ServiceDiscoveryManager.
      * @return the ServiceDiscoveryManager associated with a given Connection.
      */
-    public static ServiceDiscoveryManager getInstanceFor(Connection connection) {
+    public static synchronized ServiceDiscoveryManager getInstanceFor(Connection connection) {
-        return instances.get(connection);
+        ServiceDiscoveryManager sdm = instances.get(connection);
+        if (sdm == null) {
+            sdm = new ServiceDiscoveryManager(connection);
+        }
+        return sdm;
     }
 
     /**
@@ -173,122 +266,6 @@ public class ServiceDiscoveryManager {
         return Collections.unmodifiableList(identities);
     }
 
-    /**
-     * Initializes the packet listeners of the connection that will answer to any
-     * service discovery request. 
-     */
-    private void init() {
-        // Register the new instance and associate it with the connection 
-        instances.put(connection, this);
-
-        addFeature(DiscoverInfo.NAMESPACE);
-        addFeature(DiscoverItems.NAMESPACE);
-
-        // Add a listener to the connection that removes the registered instance when
-        // the connection is closed
-        connection.addConnectionListener(new ConnectionListener() {
-            public void connectionClosed() {
-                // Unregister this instance since the connection has been closed
-                instances.remove(connection);
-            }
-
-            public void connectionClosedOnError(Exception e) {
-                // ignore
-            }
-
-            public void reconnectionFailed(Exception e) {
-                // ignore
-            }
-
-            public void reconnectingIn(int seconds) {
-                // ignore
-            }
-
-            public void reconnectionSuccessful() {
-                // ignore
-            }
-        });
-
-        // Listen for disco#items requests and answer with an empty result        
-        PacketFilter packetFilter = new PacketTypeFilter(DiscoverItems.class);
-        PacketListener packetListener = new PacketListener() {
-            public void processPacket(Packet packet) {
-                DiscoverItems discoverItems = (DiscoverItems) packet;
-                // Send back the items defined in the client if the request is of type GET
-                if (discoverItems != null && discoverItems.getType() == IQ.Type.GET) {
-                    DiscoverItems response = new DiscoverItems();
-                    response.setType(IQ.Type.RESULT);
-                    response.setTo(discoverItems.getFrom());
-                    response.setPacketID(discoverItems.getPacketID());
-                    response.setNode(discoverItems.getNode());
-
-                    // Add the defined items related to the requested node. Look for 
-                    // the NodeInformationProvider associated with the requested node.  
-                    NodeInformationProvider nodeInformationProvider =
-                            getNodeInformationProvider(discoverItems.getNode());
-                    if (nodeInformationProvider != null) {
-                        // Specified node was found, add node items
-                        response.addItems(nodeInformationProvider.getNodeItems());
-                        // Add packet extensions
-                        response.addExtensions(nodeInformationProvider.getNodePacketExtensions());
-                    } else if(discoverItems.getNode() != null) {
-                        // Return <item-not-found/> error since client doesn't contain
-                        // the specified node
-                        response.setType(IQ.Type.ERROR);
-                        response.setError(new XMPPError(XMPPError.Condition.item_not_found));
-                    }
-                    connection.sendPacket(response);
-                }
-            }
-        };
-        connection.addPacketListener(packetListener, packetFilter);
-
-        // Listen for disco#info requests and answer the client's supported features 
-        // To add a new feature as supported use the #addFeature message        
-        packetFilter = new PacketTypeFilter(DiscoverInfo.class);
-        packetListener = new PacketListener() {
-            public void processPacket(Packet packet) {
-                DiscoverInfo discoverInfo = (DiscoverInfo) packet;
-                // Answer the client's supported features if the request is of the GET type
-                if (discoverInfo != null && discoverInfo.getType() == IQ.Type.GET) {
-                    DiscoverInfo response = new DiscoverInfo();
-                    response.setType(IQ.Type.RESULT);
-                    response.setTo(discoverInfo.getFrom());
-                    response.setPacketID(discoverInfo.getPacketID());
-                    response.setNode(discoverInfo.getNode());
-                    // Add the client's identity and features only if "node" is null
-                    // and if the request was not send to a node. If Entity Caps are
-                    // enabled the client's identity and features are may also added
-                    // if the right node is chosen
-                    if (discoverInfo.getNode() == null) {
-                        addDiscoverInfoTo(response);
-                    }
-                    else {
-                        // Disco#info was sent to a node. Check if we have information of the
-                        // specified node
-                        NodeInformationProvider nodeInformationProvider =
-                                getNodeInformationProvider(discoverInfo.getNode());
-                        if (nodeInformationProvider != null) {
-                            // Node was found. Add node features
-                            response.addFeatures(nodeInformationProvider.getNodeFeatures());
-                            // Add node identities
-                            response.addIdentities(nodeInformationProvider.getNodeIdentities());
-                            // Add packet extensions
-                            response.addExtensions(nodeInformationProvider.getNodePacketExtensions());
-                        }
-                        else {
-                            // Return <item-not-found/> error since specified node was not found
-                            response.setType(IQ.Type.ERROR);
-                            response.setError(new XMPPError(XMPPError.Condition.item_not_found));
-                        }
-                    }
-                    connection.sendPacket(response);
-                }
-            }
-        };
-        connection.addPacketListener(packetListener, packetFilter);
-    }
-
     /**
      * Add discover info response data.
      * 
@@ -534,6 +511,9 @@ public class ServiceDiscoveryManager {
      * @throws XMPPException if the operation failed for some reason.
      */
     public DiscoverInfo discoverInfo(String entityID, String node) throws XMPPException {
+        Connection connection = ServiceDiscoveryManager.this.connection.get();
+        if (connection == null) throw new XMPPException("Connection instance already gc'ed");
+
         // Discover the entity's info
         DiscoverInfo disco = new DiscoverInfo();
         disco.setType(IQ.Type.GET);
@@ -581,6 +561,9 @@ public class ServiceDiscoveryManager {
      * @throws XMPPException if the operation failed for some reason.
      */
     public DiscoverItems discoverItems(String entityID, String node) throws XMPPException {
+        Connection connection = ServiceDiscoveryManager.this.connection.get();
+        if (connection == null) throw new XMPPException("Connection instance already gc'ed");
+
         // Discover the entity's items
         DiscoverItems disco = new DiscoverItems();
         disco.setType(IQ.Type.GET);
@@ -662,6 +645,9 @@ public class ServiceDiscoveryManager {
      */
     public void publishItems(String entityID, String node, DiscoverItems discoverItems)
             throws XMPPException {
+        Connection connection = ServiceDiscoveryManager.this.connection.get();
+        if (connection == null) throw new XMPPException("Connection instance already gc'ed");
+
         discoverItems.setType(IQ.Type.SET);
         discoverItems.setTo(entityID);
         discoverItems.setNode(node);

source/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java

@@ -95,21 +95,27 @@ public class InBandBytestreamManager implements BytestreamManager {
      */
     static {
         Connection.addConnectionCreationListener(new ConnectionCreationListener() {
-            public void connectionCreated(Connection connection) {
+            public void connectionCreated(final Connection connection) {
-                final InBandBytestreamManager manager;
+                // create the manager for this connection
-                manager = InBandBytestreamManager.getByteStreamManager(connection);
+                InBandBytestreamManager.getByteStreamManager(connection);
 
                 // register shutdown listener
                 connection.addConnectionListener(new AbstractConnectionListener() {
 
                     @Override
                     public void connectionClosed() {
-                        manager.disableService();
+                        InBandBytestreamManager.getByteStreamManager(connection).disableService();
                     }
 
                     @Override
-                    public void connectionClosedOnError(Exception e) {
+                    public void connectionClosedOnError(Exception e) {
-                        manager.disableService();
+                        InBandBytestreamManager.getByteStreamManager(connection).disableService();
+                    }
+
+                    @Override
+                    public void reconnectionSuccessful() {
+                        // re-create the manager for this connection
+                        InBandBytestreamManager.getByteStreamManager(connection);
                     }
 
                 });
@@ -526,7 +532,7 @@ public class InBandBytestreamManager implements BytestreamManager {
 
     /**
      * Disables the InBandBytestreamManager by removing its packet listeners and resetting its
-     * internal status.
+     * internal status, which includes removing this instance from the managers map.
      */
     private void disableService() {
 

source/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java

@@ -89,21 +89,27 @@ public final class Socks5BytestreamManager implements BytestreamManager {
     static {
         Connection.addConnectionCreationListener(new ConnectionCreationListener() {
 
-            public void connectionCreated(Connection connection) {
+            public void connectionCreated(final Connection connection) {
-                final Socks5BytestreamManager manager;
+                // create the manager for this connection
-                manager = Socks5BytestreamManager.getBytestreamManager(connection);
+                Socks5BytestreamManager.getBytestreamManager(connection);
 
                 // register shutdown listener
                 connection.addConnectionListener(new AbstractConnectionListener() {
 
                     @Override
                     public void connectionClosed() {
-                        manager.disableService();
+                        Socks5BytestreamManager.getBytestreamManager(connection).disableService();
                     }
 
                     @Override
                     public void connectionClosedOnError(Exception e) {
-                        manager.disableService();
+                        Socks5BytestreamManager.getBytestreamManager(connection).disableService();
+                    }
+
+                    @Override
+                    public void reconnectionSuccessful() {
+                        // re-create the manager for this connection
+                        Socks5BytestreamManager.getBytestreamManager(connection);
                     }
 
                 });
@@ -274,7 +280,7 @@ public final class Socks5BytestreamManager implements BytestreamManager {
     /**
      * Disables the SOCKS5 Bytestream manager by removing the SOCKS5 Bytestream feature from the
      * service discovery, disabling the listener for SOCKS5 Bytestream initiation requests and
-     * resetting its internal state.
+     * resetting its internal state, which includes removing this instance from the managers map.
      * <p>
      * To re-enable the SOCKS5 Bytestream feature invoke {@link #getBytestreamManager(Connection)}.
      * Using the file transfer API will automatically re-enable the SOCKS5 Bytestream feature.

source/org/jivesoftware/smackx/commands/AdHocCommandManager.java

@@ -38,10 +38,13 @@ import org.jivesoftware.smackx.packet.DiscoverInfo;
 import org.jivesoftware.smackx.packet.DiscoverInfo.Identity;
 import org.jivesoftware.smackx.packet.DiscoverItems;
 
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.WeakHashMap;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
@@ -70,7 +73,7 @@ public class AdHocCommandManager {
      * pair for every active connection.
      */
     private static Map<Connection, AdHocCommandManager> instances =
-            new ConcurrentHashMap<Connection, AdHocCommandManager>();
+            Collections.synchronizedMap(new WeakHashMap<Connection, AdHocCommandManager>());
 
     /**
      * Register the listener for all the connection creations. When a new
@@ -92,26 +95,23 @@ public class AdHocCommandManager {
      * @param connection the XMPP connection.
      * @return the AdHocCommandManager associated with the connection.
      */
-    public static AdHocCommandManager getAddHocCommandsManager(Connection connection) {
+    public static synchronized AdHocCommandManager getAddHocCommandsManager(Connection connection) {
-        return instances.get(connection);
+        AdHocCommandManager ahcm = instances.get(connection);
+        if (ahcm == null) ahcm = new AdHocCommandManager(connection);
+        return ahcm;
     }
 
-    /**
-     * Thread that reaps stale sessions.
-     */
-    private Thread sessionsSweeper;
-
     /**
      * The Connection that this instances of AdHocCommandManager manages
      */
-    private Connection connection;
+    private final WeakReference<Connection> connection;
 
     /**
      * Map a command node with its AdHocCommandInfo. Note: Key=command node,
      * Value=command. Command node matches the node attribute sent by command
      * requesters.
      */
-    private Map<String, AdHocCommandInfo> commands = new ConcurrentHashMap<String, AdHocCommandInfo>();
+    private final Map<String, AdHocCommandInfo> commands = new ConcurrentHashMap<String, AdHocCommandInfo>();
 
     /**
      * Map a command session ID with the instance LocalCommand. The LocalCommand
@@ -119,175 +119,22 @@ public class AdHocCommandManager {
      * the command execution. Note: Key=session ID, Value=LocalCommand. Session
      * ID matches the sessionid attribute sent by command responders.
      */
-    private Map<String, LocalCommand> executingCommands = new ConcurrentHashMap<String, LocalCommand>();
+    private final Map<String, LocalCommand> executingCommands = new ConcurrentHashMap<String, LocalCommand>();
+    
+    private final ServiceDiscoveryManager serviceDiscoveryManager;
+    
+    /**
+     * Thread that reaps stale sessions.
+     */
+    private Thread sessionsSweeper;
 
     private AdHocCommandManager(Connection connection) {
-        super();
+        this.connection = new WeakReference<Connection>(connection);
-        this.connection = connection;
+        this.serviceDiscoveryManager = ServiceDiscoveryManager.getInstanceFor(connection);
-        init();
+        
-    }
-
-    /**
-     * Registers a new command with this command manager, which is related to a
-     * connection. The <tt>node</tt> is an unique identifier of that command for
-     * the connection related to this command manager. The <tt>name</tt> is the
-     * human readable name of the command. The <tt>class</tt> is the class of
-     * the command, which must extend {@link LocalCommand} and have a default
-     * constructor.
-     *
-     * @param node the unique identifier of the command.
-     * @param name the human readable name of the command.
-     * @param clazz the class of the command, which must extend {@link LocalCommand}.
-     */
-    public void registerCommand(String node, String name, final Class<? extends LocalCommand> clazz) {
-        registerCommand(node, name, new LocalCommandFactory() {
-            public LocalCommand getInstance() throws InstantiationException, IllegalAccessException  {
-                return clazz.newInstance();
-            }
-        });
-    }
-
-    /**
-     * Registers a new command with this command manager, which is related to a
-     * connection. The <tt>node</tt> is an unique identifier of that
-     * command for the connection related to this command manager. The <tt>name</tt>
-     * is the human readeale name of the command. The <tt>factory</tt> generates
-     * new instances of the command.
-     *
-     * @param node the unique identifier of the command.
-     * @param name the human readable name of the command.
-     * @param factory a factory to create new instances of the command.
-     */
-    public void registerCommand(String node, final String name, LocalCommandFactory factory) {
-        AdHocCommandInfo commandInfo = new AdHocCommandInfo(node, name, connection.getUser(), factory);
-
-        commands.put(node, commandInfo);
-        // Set the NodeInformationProvider that will provide information about
-        // the added command
-        ServiceDiscoveryManager.getInstanceFor(connection).setNodeInformationProvider(node,
-                new NodeInformationProvider() {
-                    public List<DiscoverItems.Item> getNodeItems() {
-                        return null;
-                    }
-
-                    public List<String> getNodeFeatures() {
-                        List<String> answer = new ArrayList<String>();
-                        answer.add(DISCO_NAMESPACE);
-                        // TODO: check if this service is provided by the
-                        // TODO: current connection.
-                        answer.add("jabber:x:data");
-                        return answer;
-                    }
-
-                    public List<DiscoverInfo.Identity> getNodeIdentities() {
-                        List<DiscoverInfo.Identity> answer = new ArrayList<DiscoverInfo.Identity>();
-                        DiscoverInfo.Identity identity = new DiscoverInfo.Identity(
-                                "automation", name, "command-node");
-                        answer.add(identity);
-                        return answer;
-                    }
-
-                    @Override
-                    public List<PacketExtension> getNodePacketExtensions() {
-                        return null;
-                    }
-
-                });
-    }
-
-    /**
-     * Discover the commands of an specific JID. The <code>jid</code> is a
-     * full JID.
-     *
-     * @param jid the full JID to retrieve the commands for.
-     * @return the discovered items.
-     * @throws XMPPException if the operation failed for some reason.
-     */
-    public DiscoverItems discoverCommands(String jid) throws XMPPException {
-        ServiceDiscoveryManager serviceDiscoveryManager = ServiceDiscoveryManager
-                .getInstanceFor(connection);
-        return serviceDiscoveryManager.discoverItems(jid, discoNode);
-    }
-
-    /**
-     * Publish the commands to an specific JID.
-     *
-     * @param jid the full JID to publish the commands to.
-     * @throws XMPPException if the operation failed for some reason.
-     */
-    public void publishCommands(String jid) throws XMPPException {
-        ServiceDiscoveryManager serviceDiscoveryManager = ServiceDiscoveryManager
-                .getInstanceFor(connection);
-
-        // Collects the commands to publish as items
-        DiscoverItems discoverItems = new DiscoverItems();
-        Collection<AdHocCommandInfo> xCommandsList = getRegisteredCommands();
-
-        for (AdHocCommandInfo info : xCommandsList) {
-            DiscoverItems.Item item = new DiscoverItems.Item(info.getOwnerJID());
-            item.setName(info.getName());
-            item.setNode(info.getNode());
-            discoverItems.addItem(item);
-        }
-
-        serviceDiscoveryManager.publishItems(jid, discoNode, discoverItems);
-    }
-
-    /**
-     * Returns a command that represents an instance of a command in a remote
-     * host. It is used to execute remote commands. The concept is similar to
-     * RMI. Every invocation on this command is equivalent to an invocation in
-     * the remote command.
-     *
-     * @param jid the full JID of the host of the remote command
-     * @param node the identifier of the command
-     * @return a local instance equivalent to the remote command.
-     */
-    public RemoteCommand getRemoteCommand(String jid, String node) {
-        return new RemoteCommand(connection, node, jid);
-    }
-
-    /**
-     * <ul>
-     * <li>Adds listeners to the connection</li>
-     * <li>Registers the ad-hoc command feature to the ServiceDiscoveryManager</li>
-     * <li>Registers the items of the feature</li>
-     * <li>Adds packet listeners to handle execution requests</li>
-     * <li>Creates and start the session sweeper</li>
-     * </ul>
-     */
-    private void init() {
         // Register the new instance and associate it with the connection
         instances.put(connection, this);
 
-        // Add a listener to the connection that removes the registered instance
-        // when the connection is closed
-        connection.addConnectionListener(new ConnectionListener() {
-            public void connectionClosed() {
-                // Unregister this instance since the connection has been closed
-                instances.remove(connection);
-            }
-
-            public void connectionClosedOnError(Exception e) {
-                // Unregister this instance since the connection has been closed
-                instances.remove(connection);
-            }
-
-            public void reconnectionSuccessful() {
-                // Register this instance since the connection has been
-                // reestablished
-                instances.put(connection, AdHocCommandManager.this);
-            }
-
-            public void reconnectingIn(int seconds) {
-                // Nothing to do
-            }
-
-            public void reconnectionFailed(Exception e) {
-                // Nothing to do
-            }
-        });
-
         // Add the feature to the service discovery manage to show that this
         // connection supports the AdHoc-Commands protocol.
         // This information will be used when another client tries to
@@ -346,6 +193,121 @@ public class AdHocCommandManager {
         sessionsSweeper = null;
     }
 
+    /**
+     * Registers a new command with this command manager, which is related to a
+     * connection. The <tt>node</tt> is an unique identifier of that command for
+     * the connection related to this command manager. The <tt>name</tt> is the
+     * human readable name of the command. The <tt>class</tt> is the class of
+     * the command, which must extend {@link LocalCommand} and have a default
+     * constructor.
+     *
+     * @param node the unique identifier of the command.
+     * @param name the human readable name of the command.
+     * @param clazz the class of the command, which must extend {@link LocalCommand}.
+     */
+    public void registerCommand(String node, String name, final Class<? extends LocalCommand> clazz) {
+        registerCommand(node, name, new LocalCommandFactory() {
+            public LocalCommand getInstance() throws InstantiationException, IllegalAccessException  {
+                return clazz.newInstance();
+            }
+        });
+    }
+
+    /**
+     * Registers a new command with this command manager, which is related to a
+     * connection. The <tt>node</tt> is an unique identifier of that
+     * command for the connection related to this command manager. The <tt>name</tt>
+     * is the human readeale name of the command. The <tt>factory</tt> generates
+     * new instances of the command.
+     *
+     * @param node the unique identifier of the command.
+     * @param name the human readable name of the command.
+     * @param factory a factory to create new instances of the command.
+     */
+    public void registerCommand(String node, final String name, LocalCommandFactory factory) {
+        AdHocCommandInfo commandInfo = new AdHocCommandInfo(node, name, connection.get().getUser(), factory);
+
+        commands.put(node, commandInfo);
+        // Set the NodeInformationProvider that will provide information about
+        // the added command
+        serviceDiscoveryManager.setNodeInformationProvider(node,
+                new NodeInformationProvider() {
+                    public List<DiscoverItems.Item> getNodeItems() {
+                        return null;
+                    }
+
+                    public List<String> getNodeFeatures() {
+                        List<String> answer = new ArrayList<String>();
+                        answer.add(DISCO_NAMESPACE);
+                        // TODO: check if this service is provided by the
+                        // TODO: current connection.
+                        answer.add("jabber:x:data");
+                        return answer;
+                    }
+
+                    public List<DiscoverInfo.Identity> getNodeIdentities() {
+                        List<DiscoverInfo.Identity> answer = new ArrayList<DiscoverInfo.Identity>();
+                        DiscoverInfo.Identity identity = new DiscoverInfo.Identity(
+                                "automation", name, "command-node");
+                        answer.add(identity);
+                        return answer;
+                    }
+
+                    @Override
+                    public List<PacketExtension> getNodePacketExtensions() {
+                        return null;
+                    }
+
+                });
+    }
+
+    /**
+     * Discover the commands of an specific JID. The <code>jid</code> is a
+     * full JID.
+     *
+     * @param jid the full JID to retrieve the commands for.
+     * @return the discovered items.
+     * @throws XMPPException if the operation failed for some reason.
+     */
+    public DiscoverItems discoverCommands(String jid) throws XMPPException {
+        return serviceDiscoveryManager.discoverItems(jid, discoNode);
+    }
+
+    /**
+     * Publish the commands to an specific JID.
+     *
+     * @param jid the full JID to publish the commands to.
+     * @throws XMPPException if the operation failed for some reason.
+     */
+    public void publishCommands(String jid) throws XMPPException {
+        // Collects the commands to publish as items
+        DiscoverItems discoverItems = new DiscoverItems();
+        Collection<AdHocCommandInfo> xCommandsList = getRegisteredCommands();
+
+        for (AdHocCommandInfo info : xCommandsList) {
+            DiscoverItems.Item item = new DiscoverItems.Item(info.getOwnerJID());
+            item.setName(info.getName());
+            item.setNode(info.getNode());
+            discoverItems.addItem(item);
+        }
+
+        serviceDiscoveryManager.publishItems(jid, discoNode, discoverItems);
+    }
+
+    /**
+     * Returns a command that represents an instance of a command in a remote
+     * host. It is used to execute remote commands. The concept is similar to
+     * RMI. Every invocation on this command is equivalent to an invocation in
+     * the remote command.
+     *
+     * @param jid the full JID of the host of the remote command
+     * @param node the identifier of the command
+     * @return a local instance equivalent to the remote command.
+     */
+    public RemoteCommand getRemoteCommand(String jid, String node) {
+        return new RemoteCommand(connection.get(), node, jid);
+    }
+
     /**
      * Process the AdHoc-Command packet that request the execution of some
      * action of a command. If this is the first request, this method checks,
@@ -491,7 +453,7 @@ public class AdHocCommandManager {
                 }
 
                 // Sends the response packet
-                connection.sendPacket(response);
+                connection.get().sendPacket(response);
 
             }
             catch (XMPPException e) {
@@ -607,7 +569,7 @@ public class AdHocCommandManager {
                         executingCommands.remove(sessionId);
                     }
 
-                    connection.sendPacket(response);
+                    connection.get().sendPacket(response);
                 }
                 catch (XMPPException e) {
                     // If there is an exception caused by the next, complete,
@@ -665,7 +627,7 @@ public class AdHocCommandManager {
     private void respondError(AdHocCommandData response, XMPPError error) {
         response.setType(IQ.Type.ERROR);
         response.setError(error);
-        connection.sendPacket(response);
+        connection.get().sendPacket(response);
     }
 
     /**

source/org/jivesoftware/smackx/entitycaps/EntityCapsManager.java

@@ -234,9 +234,7 @@ public class EntityCapsManager {
 
         connection.addConnectionListener(new ConnectionListener() {
             public void connectionClosed() {
-                // Unregister this instance since the connection has been closed
                 presenceSend = false;
-                instances.remove(weakRefConnection.get());
             }
 
             public void connectionClosedOnError(Exception e) {

source/org/jivesoftware/smackx/ping/PingManager.java

@@ -19,7 +19,6 @@ package org.jivesoftware.smackx.ping;
 import java.util.Collections;
 import java.util.Map;
 import java.util.WeakHashMap;
-import java.util.concurrent.ScheduledExecutorService;
 
 import org.jivesoftware.smack.Connection;
 import org.jivesoftware.smack.ConnectionCreationListener;
@@ -91,7 +90,7 @@ public class PingManager {
         
         // The ServiceDiscoveryManager was not pre-initialized
         if (sdm == null)
-            sdm = new ServiceDiscoveryManager(connection);
+            sdm = ServiceDiscoveryManager.getInstanceFor(connection);
         
         sdm.addFeature(Ping.NAMESPACE);
         

test-unit/org/jivesoftware/smackx/bytestreams/socks5/InitiationListenerTest.java

@@ -59,7 +59,7 @@ public class InitiationListenerTest {
         connection = mock(Connection.class);
 
         // create service discovery manager for mocked connection
-        new ServiceDiscoveryManager(connection);
+        ServiceDiscoveryManager.getInstanceFor(connection);
 
         // initialize Socks5ByteStreamManager to get the InitiationListener
         byteStreamManager = Socks5BytestreamManager.getBytestreamManager(connection);

test-unit/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java

@@ -95,8 +95,8 @@ public class Socks5ByteStreamManagerTest {
          * create service discovery managers for the connections because the
          * ConnectionCreationListener is not called when creating mocked connections
          */
-        new ServiceDiscoveryManager(connection1);
+        ServiceDiscoveryManager.getInstanceFor(connection1);
-        new ServiceDiscoveryManager(connection2);
+        ServiceDiscoveryManager.getInstanceFor(connection2);
 
         // get bytestream manager for the first connection twice
         Socks5BytestreamManager conn1ByteStreamManager1 = Socks5BytestreamManager.getBytestreamManager(connection1);

test-unit/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java

@@ -39,7 +39,7 @@ public class FileTransferNegotiatorTest {
         connection = new DummyConnection();
         connection.connect();
         connection.login("me", "secret");
-        new ServiceDiscoveryManager(connection);
+        ServiceDiscoveryManager.getInstanceFor(connection);
     }
 
     @After

test-unit/org/jivesoftware/smackx/receipts/DeliveryReceiptTest.java

@@ -70,7 +70,7 @@ public class DeliveryReceiptTest {
     @Test
     public void receiptManagerListenerTest() throws Exception {
         DummyConnection c = new DummyConnection();
-        ServiceDiscoveryManager sdm = new ServiceDiscoveryManager(c);
+        ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(c);
         DeliveryReceiptManager drm = DeliveryReceiptManager.getInstanceFor(c);
 
         TestReceiptReceivedListener rrl = new TestReceiptReceivedListener();
@@ -100,7 +100,7 @@ public class DeliveryReceiptTest {
     @Test
     public void receiptManagerAutoReplyTest() throws Exception {
         DummyConnection c = new DummyConnection();
-        ServiceDiscoveryManager sdm = new ServiceDiscoveryManager(c);
+        ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(c);
         DeliveryReceiptManager drm = DeliveryReceiptManager.getInstanceFor(c);
 
         drm.enableAutoReceipts();

test-unit/org/jivesoftware/util/ConnectionUtils.java

@@ -86,7 +86,7 @@ public class ConnectionUtils {
         when(collector.nextResult()).thenAnswer(answer);
 
         // initialize service discovery manager for this connection
-        new ServiceDiscoveryManager(connection);
+        ServiceDiscoveryManager.getInstanceFor(connection);
 
         return connection;
     }