Fixes a leak of OtrContactMenu instances (which a case of real-world usage were found to unnecessarily retain more than a megabyte of the Java heap).

src/net/java/sip/communicator/plugin/otr/OtrActivator.java

@@ -25,7 +25,7 @@ public class OtrActivator
     implements ServiceListener
 {
     /**
-     * A property used in configuration to disable the otr plugin.
+     * A property used in configuration to disable the OTR plugin.
      */
     public static final String OTR_DISABLED_PROP =
         "net.java.sip.communicator.plugin.otr.DISABLED";
@@ -37,7 +37,6 @@ public class OtrActivator
     private static final String OTR_CHAT_CONFIG_DISABLED_PROP
         = "net.java.sip.communicator.plugin.otr.otrchatconfig.DISABLED";
 
-
     /**
      * A property specifying whether private messaging should be made mandatory.
      */
@@ -71,25 +70,29 @@ public class OtrActivator
     /**
      * The {@link ResourceManagementService} of the {@link OtrActivator}. Can
      * also be obtained from the {@link OtrActivator#bundleContext} on demand,
-     * but we add it here for convinience.
+     * but we add it here for convenience.
      */
     public static ResourceManagementService resourceService;
 
     /**
      * The {@link UIService} of the {@link OtrActivator}. Can also be obtained
      * from the {@link OtrActivator#bundleContext} on demand, but we add it here
-     * for convinience.
+     * for convenience.
      */
     public static UIService uiService;
 
     /**
      * The {@link ConfigurationService} of the {@link OtrActivator}. Can also be
      * obtained from the {@link OtrActivator#bundleContext} on demand, but we
-     * add it here for convinience.
+     * add it here for convenience.
      */
     public static ConfigurationService configService;
 
-    private static Logger logger = Logger.getLogger(OtrActivator.class);
+    /**
+     * The <tt>Logger</tt> used by the <tt>OtrActivator</tt> class and its
+     * instances for logging output.
+     */
+    private static final Logger logger = Logger.getLogger(OtrActivator.class);
 
     /*
      * Implements AbstractServiceDependentActivator#start(UIService).
@@ -97,33 +100,31 @@ public class OtrActivator
     @Override
     public void start(Object dependentService)
     {
-        uiService = (UIService)dependentService;
-
-        ServiceReference refConfigService =
-            OtrActivator.bundleContext
-                .getServiceReference(ConfigurationService.class.getName());
-
-        if (refConfigService == null)
+        configService
+            = ServiceUtils.getService(
+                    bundleContext,
+                    ConfigurationService.class);
+        // Check whether someone has disabled this plug-in.
+        if(configService.getBoolean(OTR_DISABLED_PROP, false))
+        {
+            configService = null;
             return;
+        }
 
-        configService =
-            (ConfigurationService) OtrActivator.bundleContext
-                .getService(refConfigService);
-
-        // check whether someone has disabled this plugin
-        if(configService.getBoolean(OTR_DISABLED_PROP, false))
+        resourceService
+            = ResourceManagementServiceUtils.getService(bundleContext);
+        if (resourceService == null)
+        {
+            configService = null;
             return;
+        }
+
+        uiService = (UIService) dependentService;
 
         // Init static variables, don't proceed without them.
         scOtrEngine = new ScOtrEngineImpl();
         otrTransformLayer = new OtrTransformLayer();
 
-        resourceService =
-            ResourceManagementServiceUtils
-                .getService(OtrActivator.bundleContext);
-        if (resourceService == null)
-            return;
-
         // Register Transformation Layer
         bundleContext.addServiceListener(this);
 
@@ -207,7 +208,7 @@ public void start(Object dependentService)
      * @return the ui service class.
      */
     @Override
-    public Class getDependentServiceClass()
+    public Class<?> getDependentServiceClass()
     {
         return UIService.class;
     }
@@ -216,7 +217,6 @@ public Class getDependentServiceClass()
      * The bundle context to use.
      * @param context the context to set.
      */
-    @Override
     public void setBundleContext(BundleContext context)
     {
         bundleContext = context;

src/net/java/sip/communicator/plugin/otr/OtrContactMenu.java

@@ -7,6 +7,7 @@
 package net.java.sip.communicator.plugin.otr;
 
 import java.awt.event.*;
+import java.lang.ref.*;
 
 import javax.swing.*;
 
@@ -20,9 +21,8 @@
  * Off-the-Record functionality for a specific contact.
  *
  * @author George Politis
- * @author Lubomir Marinov
+ * @author Lyubomir Marinov
  */
-@SuppressWarnings("serial")
 class OtrContactMenu
     implements ActionListener,
                ScOtrEngineListener,
@@ -64,9 +64,7 @@ class OtrContactMenu
 
     private final JMenu parentMenu;
 
-    private final boolean isSeparateMenu;
-
-    private SIPCommMenu separateMenu;
+    private final SIPCommMenu separateMenu;
 
     /**
      * The OtrContactMenu constructor.
@@ -84,15 +82,26 @@ public OtrContactMenu(  Contact contact,
         this.contact = contact;
         this.inMacOSXScreenMenuBar = inMacOSXScreenMenuBar;
         this.parentMenu = menu;
-        this.isSeparateMenu = isSeparateMenu;
-
-        if (isSeparateMenu)
-        {
-            separateMenu = new SIPCommMenu(contact.getDisplayName());
-        }
 
-        OtrActivator.scOtrEngine.addListener(this);
-        OtrActivator.scOtrKeyManager.addListener(this);
+        separateMenu
+            = isSeparateMenu
+                ? new SIPCommMenu(contact.getDisplayName())
+                : null;
+
+        /*
+         * XXX This OtrContactMenu instance cannot be added as a listener to
+         * scOtrEngine and scOtrKeyManager without being removed later on
+         * because the latter live forever. Unfortunately, the dispose() method
+         * of this instance is never executed. WeakListener will keep this
+         * instance as a listener of scOtrEngine and scOtrKeyManager for as long
+         * as this instance is necessary. And this instance will be strongly
+         * referenced by the JMenuItems which depict it. So when the JMenuItems
+         * are gone, this instance will become obsolete and WeakListener will
+         * remove it as a listener of scOtrEngine and scOtrKeyManager.
+         */
+        new WeakListener(
+                this,
+                OtrActivator.scOtrEngine, OtrActivator.scOtrKeyManager);
 
         setSessionStatus(OtrActivator.scOtrEngine.getSessionStatus(contact));
         setOtrPolicy(OtrActivator.scOtrEngine.getContactPolicy(contact));
@@ -199,8 +208,7 @@ void dispose()
      */
     public void globalPolicyChanged()
     {
-        setOtrPolicy(OtrActivator.scOtrEngine
-            .getContactPolicy(OtrContactMenu.this.contact));
+        setOtrPolicy(OtrActivator.scOtrEngine.getContactPolicy(contact));
     }
 
     /**
@@ -240,7 +248,7 @@ private void buildMenu()
             authBuddy.setActionCommand(ACTION_COMMAND_AUTHENTICATE_BUDDY);
             authBuddy.addActionListener(this);
 
-            if (isSeparateMenu)
+            if (separateMenu != null)
             {
                 separateMenu.add(endOtr);
                 separateMenu.add(refreshOtr);
@@ -256,7 +264,7 @@ private void buildMenu()
             break;
 
         case FINISHED:
-            if (isSeparateMenu)
+            if (separateMenu != null)
             {
                 separateMenu.add(endOtr);
                 separateMenu.add(startOtr);
@@ -269,7 +277,7 @@ private void buildMenu()
             break;
 
         case PLAINTEXT:
-            if (isSeparateMenu)
+            if (separateMenu != null)
                 separateMenu.add(startOtr);
             else
                 parentMenu.add(startOtr);
@@ -330,7 +338,7 @@ else if (!isMandatory && defaultOtrPropValue != null)
         cbReset.setActionCommand(ACTION_COMMAND_CB_RESET);
         cbReset.addActionListener(this);
 
-        if (isSeparateMenu)
+        if (separateMenu != null)
         {
             separateMenu.addSeparator();
             separateMenu.add(cbEnable);
@@ -377,7 +385,6 @@ private void setSessionStatus(SessionStatus sessionStatus)
             if (separateMenu != null)
             {
                 updateIcon();
-
                 if (separateMenu.isPopupMenuVisible() || inMacOSXScreenMenuBar)
                     buildMenu();
             }
@@ -396,9 +403,12 @@ private void setOtrPolicy(OtrPolicy otrPolicy)
         {
             this.otrPolicy = otrPolicy;
 
-            if (separateMenu != null
-                && (separateMenu.isPopupMenuVisible() || inMacOSXScreenMenuBar))
+            if ((separateMenu != null)
+                    && (separateMenu.isPopupMenuVisible()
+                            || inMacOSXScreenMenuBar))
+            {
                 buildMenu();
+            }
         }
     }
 
@@ -408,7 +418,7 @@ private void setOtrPolicy(OtrPolicy otrPolicy)
      */
     private void updateIcon()
     {
-        if (!isSeparateMenu)
+        if (separateMenu == null)
             return;
 
         String imageID;
@@ -436,4 +446,147 @@ private void updateIcon()
 
         separateMenu.setIcon(OtrActivator.resourceService.getImage(imageID));
     }
+
+    /**
+     * Implements a <tt>ScOtrEngineListener</tt> and
+     * <tt>ScOtrKeyManagerListener</tt> listener for the purposes of
+     * <tt>OtrContactMenu</tt> which listens to <tt>ScOtrEngine</tt> and
+     * <tt>ScOtrKeyManager</tt> while weakly referencing the
+     * <tt>OtrContactMenu</tt>. Fixes a memory leak of <tt>OtrContactMenu</tt>
+     * instances because these cannot determine when they are to be explicitly
+     * disposed.
+     *
+     * @author Lyubomir Marinov
+     */
+    private static class WeakListener
+        implements ScOtrEngineListener,
+                   ScOtrKeyManagerListener
+    {
+        /**
+         * The <tt>ScOtrEngine</tt> the <tt>OtrContactMenu</tt> associated with
+         * this instance is to listen to.
+         */
+        private final ScOtrEngine engine;
+
+        /**
+         * The <tt>ScOtrKeyManager</tt> the <tt>OtrContactMenu</tt> associated
+         * with this instance is to listen to.
+         */
+        private final ScOtrKeyManager keyManager;
+
+        /**
+         * The <tt>OtrContactMenu</tt> which is associated with this instance
+         * and which is to listen to {@link #engine} and {@link #keyManager}. 
+         */
+        private final WeakReference<OtrContactMenu> listener;
+
+        /**
+         * Initializes a new <tt>WeakListener</tt> instance which is to allow
+         * a specific <tt>OtrContactMenu</tt> to listener to a specific
+         * <tt>ScOtrEngine</tt> and a specific <tt>ScOtrKeyManager</tt> without
+         * being retained by them forever (because they live forever).
+         *
+         * @param listener the <tt>OtrContactMenu</tt> which is to listen to the
+         * specified <tt>engine</tt> and <tt>keyManager</tt>
+         * @param engine the <tt>ScOtrEngine</tt> which is to be listened to by
+         * the specified <tt>OtrContactMenu</tt>
+         * @param keyManager the <tt>ScOtrKeyManager</tt> which is to be
+         * listened to by the specified <tt>OtrContactMenu</tt>
+         */
+        public WeakListener(
+                OtrContactMenu listener,
+                ScOtrEngine engine, ScOtrKeyManager keyManager)
+        {
+            if (listener == null)
+                throw new NullPointerException("listener");
+
+            this.listener = new WeakReference<OtrContactMenu>(listener);
+            this.engine = engine;
+            this.keyManager = keyManager;
+
+            this.engine.addListener(this);
+            this.keyManager.addListener(this);
+        }
+
+        /**
+         * {@inheritDoc}
+         *
+         * Forwards the event/notification to the associated
+         * <tt>OtrContactMenu</tt> if it is still needed by the application.
+         */
+        public void contactPolicyChanged(Contact contact)
+        {
+            ScOtrEngineListener l = getListener();
+
+            if (l != null)
+                l.contactPolicyChanged(contact);
+        }
+
+        /**
+         * {@inheritDoc}
+         *
+         * Forwards the event/notification to the associated
+         * <tt>OtrContactMenu</tt> if it is still needed by the application.
+         */
+        public void contactVerificationStatusChanged(Contact contact)
+        {
+            ScOtrKeyManagerListener l = getListener();
+
+            if (l != null)
+                l.contactVerificationStatusChanged(contact);
+        }
+
+        /**
+         * Gets the <tt>OtrContactMenu</tt> which is listening to
+         * {@link #engine} and {@link #keyManager}. If the
+         * <tt>OtrContactMenu</tt> is no longer needed by the application, this
+         * instance seizes listening to <tt>engine</tt> and <tt>keyManager</tt>
+         * and allows the memory used by this instance to be reclaimed by the
+         * Java virtual machine.
+         *
+         * @return the <tt>OtrContactMenu</tt> which is listening to
+         * <tt>engine</tt> and <tt>keyManager</tt> if it is still needed by the
+         * application; otherwise, <tt>null</tt>
+         */
+        private OtrContactMenu getListener()
+        {
+            OtrContactMenu l = this.listener.get();
+
+            if (l == null)
+            {
+                engine.removeListener(this);
+                keyManager.removeListener(this);
+            }
+
+            return l;
+        }
+
+        /**
+         * {@inheritDoc}
+         *
+         * Forwards the event/notification to the associated
+         * <tt>OtrContactMenu</tt> if it is still needed by the application.
+         */
+        public void globalPolicyChanged()
+        {
+            ScOtrEngineListener l = getListener();
+
+            if (l != null)
+                l.globalPolicyChanged();
+        }
+
+        /**
+         * {@inheritDoc}
+         *
+         * Forwards the event/notification to the associated
+         * <tt>OtrContactMenu</tt> if it is still needed by the application.
+         */
+        public void sessionStatusChanged(Contact contact)
+        {
+            ScOtrEngineListener l = getListener();
+
+            if (l != null)
+                l.sessionStatusChanged(contact);
+        }
+    }
 }

src/net/java/sip/communicator/plugin/otr/OtrMetaContactMenu.java

@@ -183,10 +183,12 @@ public void popupMenuWillBecomeVisible(PopupMenuEvent e)
         menu.addSeparator();
 
         whatsThis = new JMenuItem();
-        whatsThis.setIcon(OtrActivator.resourceService
-            .getImage("plugin.otr.HELP_ICON_15x15"));
-        whatsThis.setText(OtrActivator.resourceService
-            .getI18NString("plugin.otr.menu.WHATS_THIS"));
+        whatsThis.setIcon(
+                OtrActivator.resourceService.getImage(
+                        "plugin.otr.HELP_ICON_15x15"));
+        whatsThis.setText(
+                OtrActivator.resourceService.getI18NString(
+                        "plugin.otr.menu.WHATS_THIS"));
         whatsThis.addActionListener(this);
         menu.add(whatsThis);
     }

src/net/java/sip/communicator/plugin/otr/ScOtrEngineImpl.java

@@ -19,7 +19,9 @@
 import org.osgi.framework.*;
 
 /**
+ *
  * @author George Politis
+ * @author Lyubomir Marinov
  */
 public class ScOtrEngineImpl
     implements ScOtrEngine,
@@ -47,6 +49,23 @@ public void addListener(ScOtrEngineListener l)
         }
     }
 
+    /**
+     * Gets a copy of the list of <tt>ScOtrEngineListener</tt>s registered with
+     * this instance which may safely be iterated without the risk of a
+     * <tt>ConcurrentModificationException</tt>.
+     *
+     * @return a copy of the list of <tt>ScOtrEngineListener<tt>s registered
+     * with this instance which may safely be iterated without the risk of a
+     * <tt>ConcurrentModificationException</tt>
+     */
+    private ScOtrEngineListener[] getListeners()
+    {
+        synchronized (listeners)
+        {
+            return listeners.toArray(new ScOtrEngineListener[listeners.size()]);
+        }
+    }
+
     public void removeListener(ScOtrEngineListener l)
     {
         synchronized (listeners)
@@ -210,10 +229,8 @@ public void sessionStatusChanged(SessionID sessionID)
                     Chat.SYSTEM_MESSAGE, message,
                     OperationSetBasicInstantMessaging.HTML_MIME_TYPE);
 
-                for (ScOtrEngineListener l : listeners)
-                {
+                for (ScOtrEngineListener l : getListeners())
                     l.sessionStatusChanged(contact);
-                }
             }
         });
     }
@@ -327,7 +344,7 @@ public void setGlobalPolicy(OtrPolicy policy)
         else
             this.configurator.setProperty("POLICY", policy.getPolicy());
 
-        for (ScOtrEngineListener l : listeners)
+        for (ScOtrEngineListener l : getListeners())
             l.globalPolicyChanged();
     }
 
@@ -366,7 +383,7 @@ public void setContactPolicy(Contact contact, OtrPolicy policy)
         else
             this.configurator.setProperty(propertyID, policy.getPolicy());
 
-        for (ScOtrEngineListener l : listeners)
+        for (ScOtrEngineListener l : getListeners())
             l.contactPolicyChanged(contact);
     }
 

src/net/java/sip/communicator/plugin/otr/ScOtrEngineListener.java

@@ -9,15 +9,14 @@
 import net.java.sip.communicator.service.protocol.*;
 
 /**
- * 
+ *
  * @author George Politis
- * 
  */
 public interface ScOtrEngineListener
 {
-    public abstract void sessionStatusChanged(Contact contact);
+    public void contactPolicyChanged(Contact contact);
 
-    public abstract void contactPolicyChanged(Contact contact);
+    public void globalPolicyChanged();
 
-    public abstract void globalPolicyChanged();
+    public void sessionStatusChanged(Contact contact);
 }

src/net/java/sip/communicator/plugin/otr/ScOtrKeyManagerImpl.java

@@ -14,7 +14,9 @@
 import net.java.sip.communicator.service.protocol.*;
 
 /**
+ *
  * @author George Politis
+ * @author Lyubomir Marinov
  */
 public class ScOtrKeyManagerImpl
     implements ScOtrKeyManager
@@ -33,6 +35,25 @@ public void addListener(ScOtrKeyManagerListener l)
         }
     }
 
+    /**
+     * Gets a copy of the list of <tt>ScOtrKeyManagerListener</tt>s registered
+     * with this instance which may safely be iterated without the risk of a
+     * <tt>ConcurrentModificationException</tt>.
+     *
+     * @return a copy of the list of <tt>ScOtrKeyManagerListener<tt>s registered
+     * with this instance which may safely be iterated without the risk of a
+     * <tt>ConcurrentModificationException</tt>
+     */
+    private ScOtrKeyManagerListener[] getListeners()
+    {
+        synchronized (listeners)
+        {
+            return
+                listeners.toArray(
+                        new ScOtrKeyManagerListener[listeners.size()]);
+        }
+    }
+
     public void removeListener(ScOtrKeyManagerListener l)
     {
         synchronized (listeners)
@@ -49,7 +70,7 @@ public void verify(Contact contact)
         this.configurator.setProperty(contact.getAddress()
             + ".publicKey.verified", true);
 
-        for (ScOtrKeyManagerListener l : listeners)
+        for (ScOtrKeyManagerListener l : getListeners())
             l.contactVerificationStatusChanged(contact);
     }
 
@@ -61,7 +82,7 @@ public void unverify(Contact contact)
         this.configurator.removeProperty(contact.getAddress()
             + ".publicKey.verified");
 
-        for (ScOtrKeyManagerListener l : listeners)
+        for (ScOtrKeyManagerListener l : getListeners())
             l.contactVerificationStatusChanged(contact);
     }
 

src/net/java/sip/communicator/plugin/otr/ScOtrKeyManagerListener.java

@@ -9,11 +9,10 @@
 import net.java.sip.communicator.service.protocol.*;
 
 /**
- * 
- * @author George Politis
  *
+ * @author George Politis
  */
 public interface ScOtrKeyManagerListener
 {
-    public abstract void contactVerificationStatusChanged(Contact contact);
+    public void contactVerificationStatusChanged(Contact contact);
 }

src/net/java/sip/communicator/util/AbstractServiceDependentActivator.java

@@ -68,7 +68,7 @@ public void start(BundleContext bundleContext)
      * The class of the service which this activator is interested in.
      * @return the class name.
      */
-    public abstract Class getDependentServiceClass();
+    public abstract Class<?> getDependentServiceClass();
 
     /**
      * Setting context to the activator, as soon as we have one.