From fa289eac045209eab0d0ef8af9aecda74834d541 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 3 Mar 2014 11:53:55 +0100 Subject: [PATCH] Migrated Roster Item Exchange to new manager design The finalize approach was flawed anyway, it would have never been called. Because if the packetListener was still referenced from a connection, and the connection was still strong referenced, then a strong reference from a gc root would still exists to the manager, which would prevent it from being gc'ed and finalized being called. --- .../smackx/xroster/RosterExchangeManager.java | 92 +++++++++---------- .../smackx/xroster/packet/RosterExchange.java | 5 +- 2 files changed, 49 insertions(+), 48 deletions(-) diff --git a/extensions/src/main/java/org/jivesoftware/smackx/xroster/RosterExchangeManager.java b/extensions/src/main/java/org/jivesoftware/smackx/xroster/RosterExchangeManager.java index fa71b03fe..01997e75b 100644 --- a/extensions/src/main/java/org/jivesoftware/smackx/xroster/RosterExchangeManager.java +++ b/extensions/src/main/java/org/jivesoftware/smackx/xroster/RosterExchangeManager.java @@ -14,12 +14,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.jivesoftware.smackx.xroster; -import java.util.ArrayList; +import java.lang.ref.WeakReference; +import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; -import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.WeakHashMap; import org.jivesoftware.smack.PacketListener; import org.jivesoftware.smack.Roster; @@ -43,21 +46,46 @@ import org.jivesoftware.smackx.xroster.packet.RosterExchange; */ public class RosterExchangeManager { - private List rosterExchangeListeners = new ArrayList(); + public final static String NAMESPACE = "jabber:x:roster"; + public final static String ELEMENT = "x"; - private Connection con; + private final static Map INSTANCES = + Collections.synchronizedMap(new WeakHashMap()); - private PacketFilter packetFilter = new PacketExtensionFilter("x", "jabber:x:roster"); - private PacketListener packetListener; + private final static PacketFilter PACKET_FILTER = new PacketExtensionFilter(ELEMENT, NAMESPACE); + + private final Set rosterExchangeListeners = Collections.synchronizedSet(new HashSet()); + + private final WeakReference weakRefConnection; + private final PacketListener packetListener; + + public synchronized static RosterExchangeManager getInstanceFor(Connection connection) { + RosterExchangeManager rosterExchangeManager = INSTANCES.get(connection); + if (rosterExchangeManager == null) { + rosterExchangeManager = new RosterExchangeManager(connection); + } + return rosterExchangeManager; + } /** * Creates a new roster exchange manager. * * @param con a Connection which is used to send and receive messages. */ - public RosterExchangeManager(Connection con) { - this.con = con; - init(); + public RosterExchangeManager(Connection connection) { + weakRefConnection = new WeakReference(connection); + // Listens for all roster exchange packets and fire the roster exchange listeners. + packetListener = new PacketListener() { + public void processPacket(Packet packet) { + Message message = (Message) packet; + RosterExchange rosterExchange = + (RosterExchange) message.getExtension(ELEMENT, NAMESPACE); + // Fire event for roster exchange listeners + fireRosterExchangeListeners(message.getFrom(), rosterExchange.getRosterEntries()); + }; + + }; + connection.addPacketListener(packetListener, PACKET_FILTER); } /** @@ -67,11 +95,7 @@ public class RosterExchangeManager { * @param rosterExchangeListener a roster exchange listener. */ public void addRosterListener(RosterExchangeListener rosterExchangeListener) { - synchronized (rosterExchangeListeners) { - if (!rosterExchangeListeners.contains(rosterExchangeListener)) { - rosterExchangeListeners.add(rosterExchangeListener); - } - } + rosterExchangeListeners.add(rosterExchangeListener); } /** @@ -81,9 +105,7 @@ public class RosterExchangeManager { * @param rosterExchangeListener a roster exchange listener.. */ public void removeRosterListener(RosterExchangeListener rosterExchangeListener) { - synchronized (rosterExchangeListeners) { - rosterExchangeListeners.remove(rosterExchangeListener); - } + rosterExchangeListeners.remove(rosterExchangeListener); } /** @@ -100,8 +122,9 @@ public class RosterExchangeManager { RosterExchange rosterExchange = new RosterExchange(roster); msg.addExtension(rosterExchange); + Connection connection = weakRefConnection.get(); // Send the message that contains the roster - con.sendPacket(msg); + connection.sendPacket(msg); } /** @@ -118,8 +141,9 @@ public class RosterExchangeManager { rosterExchange.addRosterEntry(rosterEntry); msg.addExtension(rosterExchange); + Connection connection = weakRefConnection.get(); // Send the message that contains the roster - con.sendPacket(msg); + connection.sendPacket(msg); } /** @@ -139,8 +163,9 @@ public class RosterExchangeManager { } msg.addExtension(rosterExchange); + Connection connection = weakRefConnection.get(); // Send the message that contains the roster - con.sendPacket(msg); + connection.sendPacket(msg); } /** @@ -156,29 +181,4 @@ public class RosterExchangeManager { listeners[i].entriesReceived(from, remoteRosterEntries); } } - - private void init() { - // Listens for all roster exchange packets and fire the roster exchange listeners. - packetListener = new PacketListener() { - public void processPacket(Packet packet) { - Message message = (Message) packet; - RosterExchange rosterExchange = - (RosterExchange) message.getExtension("x", "jabber:x:roster"); - // Fire event for roster exchange listeners - fireRosterExchangeListeners(message.getFrom(), rosterExchange.getRosterEntries()); - }; - - }; - con.addPacketListener(packetListener, packetFilter); - } - - public void destroy() { - if (con != null) - con.removePacketListener(packetListener); - - } - protected void finalize() throws Throwable { - destroy(); - super.finalize(); - } } diff --git a/extensions/src/main/java/org/jivesoftware/smackx/xroster/packet/RosterExchange.java b/extensions/src/main/java/org/jivesoftware/smackx/xroster/packet/RosterExchange.java index 050560f55..3b156af76 100644 --- a/extensions/src/main/java/org/jivesoftware/smackx/xroster/packet/RosterExchange.java +++ b/extensions/src/main/java/org/jivesoftware/smackx/xroster/packet/RosterExchange.java @@ -22,6 +22,7 @@ import org.jivesoftware.smack.RosterEntry; import org.jivesoftware.smack.RosterGroup; import org.jivesoftware.smack.packet.PacketExtension; import org.jivesoftware.smackx.xroster.RemoteRosterEntry; +import org.jivesoftware.smackx.xroster.RosterExchangeManager; import java.util.ArrayList; import java.util.Collections; @@ -110,7 +111,7 @@ public class RosterExchange implements PacketExtension { * @return the XML element name of the packet extension. */ public String getElementName() { - return "x"; + return RosterExchangeManager.ELEMENT; } /** @@ -121,7 +122,7 @@ public class RosterExchange implements PacketExtension { * @return the XML namespace of the packet extension. */ public String getNamespace() { - return "jabber:x:roster"; + return RosterExchangeManager.NAMESPACE; } /**