Changeset 31669 for main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/CollectionConstructor.java

Timestamp:

2017-05-10T18:04:08+12:00 (7 years ago)

Author:

ak19

Message:

Have made the EventListeners list in CollectionConstructor use the threadsafe if less efficient CopyOnWriteArrayList, so that the send event messages called by GS2PerlConstructor are threadsafe. This reduces the need to make certain methods in GS2PerlConstructor declared as synchronized, which would have locked the GS2PerlConstructor object instance rather than its inherited listeners member variable.

File:

• main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/CollectionConstructor.java (modified) (4 diffs)

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/CollectionConstructor.java

@@ -5 +5 @@
 import org.greenstone.gsdl3.util.*;
 //import java.util.Thread
-import javax.swing.event.EventListenerList;
+import java.util.concurrent.CopyOnWriteArrayList;
 import org.w3c.dom.Element;
 
@@ -22 +22 @@
     /** other arguments/parameters for the construction process - in a paramList */
     protected Element process_params = null;
-    /** the list of listeners for the process */
-    protected EventListenerList listeners = null;
+    /** the list of listeners for the process. We need it to be threadsafe. 
+     * see http://stackoverflow.com/questions/8259479/should-i-synchronize-listener-notifications-or-not
+     * https://docs.oracle.com/javase/6/docs/api/java/util/concurrent/CopyOnWriteArrayList.html
+     * "A thread-safe variant of ArrayList in which all mutative operations (add, set, and so on) are
+     * implemented by making a fresh copy of the underlying array.
+     * This is ordinarily too costly, but may be more efficient than alternatives when traversal operations
+     * vastly outnumber mutations, and is useful when you cannot or don't want to synchronize traversals,
+     * yet need to preclude interference among concurrent threads."
+     */
+    protected final CopyOnWriteArrayList<ConstructionListener> listeners;
     /** A flag used to determine if this process has been asked to cancel. */
     protected boolean cancel = false; // Not really used (in any way that works)
@@ -34 +42 @@
     {
         super(name);
-        this.listeners = new EventListenerList();
+        this.listeners = new CopyOnWriteArrayList<ConstructionListener>();
     }
 
@@ -87 +95 @@
     public boolean addListener(ConstructionListener listener)
     {
-        this.listeners.add(ConstructionListener.class, listener);
-        return true;
+        this.listeners.add(listener);
+        return true;
     }
 
     public boolean removeListener(ConstructionListener listener)
-    {
-        this.listeners.remove(ConstructionListener.class, listener);
-        return true;
+    {       
+        this.listeners.remove(listener);
+        return true;
     }
 
     protected void sendProcessBegun(ConstructionEvent evt)
     {
-        Object[] concerned = this.listeners.getListenerList();
-        for (int i = 0; i < concerned.length; i += 2)
-        {
-            if (concerned[i] == ConstructionListener.class)
-            {
-                ((ConstructionListener) concerned[i + 1]).processBegun(evt);
-            }
-        }
+        // See http://stackoverflow.com/questions/8259479/should-i-synchronize-listener-notifications-or-not
+        for (ConstructionListener l: this.listeners) {
+        l.processBegun(evt);
+        }
     }
 
     protected void sendProcessComplete(ConstructionEvent evt)
     {
-        Object[] concerned = this.listeners.getListenerList();
-        for (int i = 0; i < concerned.length; i += 2)
-        {
-            if (concerned[i] == ConstructionListener.class)
-            {
-                ((ConstructionListener) concerned[i + 1]).processComplete(evt);
-            }
-        }
+        for (ConstructionListener l: this.listeners) {
+        l.processComplete(evt);
+        }
     }
 
-    protected synchronized void sendProcessStatus(ConstructionEvent evt)
+    // Method doesn't need to be synchronized any more, since it uses the ThreadSafe CopyOnWriteArrayList
+    // for listeners list.
+    // See http://stackoverflow.com/questions/8259479/should-i-synchronize-listener-notifications-or-not
+    // See http://stackoverflow.com/questions/574240/is-there-an-advantage-to-use-a-synchronized-method-instead-of-a-synchronized-blo
+    protected void sendProcessStatus(ConstructionEvent evt)
     {
-
-        Object[] concerned = this.listeners.getListenerList();
-        for (int i = 0; i < concerned.length; i += 2)
-        {
-            if (concerned[i] == ConstructionListener.class)
-            {
-                ((ConstructionListener) concerned[i + 1]).processStatus(evt);
-            }
-        }
+        for (ConstructionListener l: this.listeners) {
+        l.processStatus(evt);
+        }
     }
 
     protected void sendMessage(ConstructionEvent evt)
     {
-
-        Object[] concerned = this.listeners.getListenerList();
-        for (int i = 0; i < concerned.length; i += 2)
-        {
-            if (concerned[i] == ConstructionListener.class)
-            {
-                ((ConstructionListener) concerned[i + 1]).message(evt);
-            }
-        }
+        for (ConstructionListener l: this.listeners) {
+        l.message(evt);
+        }
     }