Changeset 31669 for main/trunk


Ignore:
Timestamp:
05/10/17 18:04:08 (4 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.

Location:
main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/CollectionConstructor.java

    r31667 r31669  
    55import org.greenstone.gsdl3.util.*;
    66//import java.util.Thread
    7 import javax.swing.event.EventListenerList;
     7import java.util.concurrent.CopyOnWriteArrayList;
    88import org.w3c.dom.Element;
    99
     
    2222    /** other arguments/parameters for the construction process - in a paramList */
    2323    protected Element process_params = null;
    24     /** the list of listeners for the process */
    25     protected EventListenerList listeners = null;
     24    /** the list of listeners for the process. We need it to be threadsafe.
     25     * see http://stackoverflow.com/questions/8259479/should-i-synchronize-listener-notifications-or-not
     26     * https://docs.oracle.com/javase/6/docs/api/java/util/concurrent/CopyOnWriteArrayList.html
     27     * "A thread-safe variant of ArrayList in which all mutative operations (add, set, and so on) are
     28     * implemented by making a fresh copy of the underlying array.
     29     * This is ordinarily too costly, but may be more efficient than alternatives when traversal operations
     30     * vastly outnumber mutations, and is useful when you cannot or don't want to synchronize traversals,
     31     * yet need to preclude interference among concurrent threads."
     32     */
     33    protected final CopyOnWriteArrayList<ConstructionListener> listeners;
    2634    /** A flag used to determine if this process has been asked to cancel. */
    2735    protected boolean cancel = false; // Not really used (in any way that works)
     
    3442    {
    3543        super(name);
    36         this.listeners = new EventListenerList();
     44        this.listeners = new CopyOnWriteArrayList<ConstructionListener>();
    3745    }
    3846
     
    8795    public boolean addListener(ConstructionListener listener)
    8896    {
    89         this.listeners.add(ConstructionListener.class, listener);
    90         return true;
     97        this.listeners.add(listener);
     98        return true;
    9199    }
    92100
    93101    public boolean removeListener(ConstructionListener listener)
    94     {
    95         this.listeners.remove(ConstructionListener.class, listener);
    96         return true;
     102    {       
     103        this.listeners.remove(listener);
     104        return true;
    97105    }
    98106
    99107    protected void sendProcessBegun(ConstructionEvent evt)
    100108    {
    101         Object[] concerned = this.listeners.getListenerList();
    102         for (int i = 0; i < concerned.length; i += 2)
    103         {
    104             if (concerned[i] == ConstructionListener.class)
    105             {
    106                 ((ConstructionListener) concerned[i + 1]).processBegun(evt);
    107             }
    108         }
     109        // See http://stackoverflow.com/questions/8259479/should-i-synchronize-listener-notifications-or-not
     110        for (ConstructionListener l: this.listeners) {
     111        l.processBegun(evt);
     112        }
    109113    }
    110114
    111115    protected void sendProcessComplete(ConstructionEvent evt)
    112116    {
    113         Object[] concerned = this.listeners.getListenerList();
    114         for (int i = 0; i < concerned.length; i += 2)
    115         {
    116             if (concerned[i] == ConstructionListener.class)
    117             {
    118                 ((ConstructionListener) concerned[i + 1]).processComplete(evt);
    119             }
    120         }
     117        for (ConstructionListener l: this.listeners) {
     118        l.processComplete(evt);
     119        }
    121120    }
    122121
    123     protected synchronized void sendProcessStatus(ConstructionEvent evt)
     122    // Method doesn't need to be synchronized any more, since it uses the ThreadSafe CopyOnWriteArrayList
     123    // for listeners list.
     124    // See http://stackoverflow.com/questions/8259479/should-i-synchronize-listener-notifications-or-not
     125    // See http://stackoverflow.com/questions/574240/is-there-an-advantage-to-use-a-synchronized-method-instead-of-a-synchronized-blo
     126    protected void sendProcessStatus(ConstructionEvent evt)
    124127    {
    125 
    126         Object[] concerned = this.listeners.getListenerList();
    127         for (int i = 0; i < concerned.length; i += 2)
    128         {
    129             if (concerned[i] == ConstructionListener.class)
    130             {
    131                 ((ConstructionListener) concerned[i + 1]).processStatus(evt);
    132             }
    133         }
     128        for (ConstructionListener l: this.listeners) {
     129        l.processStatus(evt);
     130        }
    134131    }
    135132
    136133    protected void sendMessage(ConstructionEvent evt)
    137134    {
    138 
    139         Object[] concerned = this.listeners.getListenerList();
    140         for (int i = 0; i < concerned.length; i += 2)
    141         {
    142             if (concerned[i] == ConstructionListener.class)
    143             {
    144                 ((ConstructionListener) concerned[i + 1]).message(evt);
    145             }
    146         }
     135        for (ConstructionListener l: this.listeners) {
     136        l.message(evt);
     137        }
    147138    }
    148139
  • main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/GS2PerlConstructor.java

    r31668 r31669  
    449449    }
    450450
    451        
     451
    452452    protected boolean runPerlCommand(String[] command, String[] envvars, File dir)
    453453    {
     
    700700    // exceptions when reading from our perl process' stderr and stdout streams are handled by
    701701    // SynchronizedProcessHandler.gotException() below, since they happen in separate threads
    702     // from this one (the one from which the perl process is run).
    703     public synchronized void gotException(Exception e) {
     702    // from this one (the one from which the perl process is run). So this method's operations
     703    // have been made thread-safe, rather than synchronizing on the method itself which locks
     704    // *this* object and which isn't actually useful for what I want to do.
     705    public void gotException(Exception e) {
    704706
    705707    // do what original runPerlCommand() code always did when an exception occurred
     
    791793
    792794        // this next method is thread safe since only synchronized methods are invoked.
    793         // and only immutable (final) vars are used. NO, What about the listeners???
    794         // So have made this next method synchronized, which synchronizes on the GS2PerlConstructor
    795         // and consequently its member variables are protected against concurrent access
    796         // by multiple threads.
     795        // and only immutable (final) vars are used. And the listeners of this class have
     796        // now been made threadsafe by using CopyOnWriteArrayList in place of EventListenerList.
    797797        sendProcessStatus(new ConstructionEvent(GS2PerlConstructor.this, GSStatus.CONTINUING, line));
    798798       
Note: See TracChangeset for help on using the changeset viewer.