Changeset 31666 for main/trunk/gli

Show
Ignore:
Timestamp:
09.05.2017 19:06:44 (3 years ago)
Author:
ak19
Message:

Uses synchronized methods vs synchronizing on objects in the right way and at the appropriate times.

Files:
1 modified

Legend:

Unmodified
Added
Removed
  • main/trunk/gli/src/org/greenstone/gatherer/shell/GShell.java

    r31639 r31666  
    357357    prcs = new SafeProcess(args); 
    358358    SafeProcess.LineByLineHandler processOutLineHandler 
    359         = new SynchronizedLineByLineHandler(bos, SafeProcess.STDOUT); 
     359        = new SynchronizedLineByLineHandler(SafeProcess.STDOUT); 
    360360    SafeProcess.LineByLineHandler processErrLineHandler 
    361         = new SynchronizedLineByLineHandler(bos, SafeProcess.STDERR); 
     361        = new SynchronizedLineByLineHandler(SafeProcess.STDERR); 
    362362     
    363363    prcs.setExceptionHandler(this);  
     
    524524    } 
    525525 
    526  
    527     public void fireMessage(String message) 
     526    // Now synchronized, since separate threads handling process out and err streams can call fireMessage() 
     527    // and need to reserve member objects and member variables used in this method while doing so. 
     528    public synchronized void fireMessage(String message) 
    528529    { 
    529530    fireMessage(type, typeAsString(type) + "> " + message, status, buffered_output_stream); 
     
    719720    protected class SynchronizedLineByLineHandler extends SafeProcess.LineByLineHandler 
    720721    { 
    721     private final BufferedOutputStream bos; // needs to be final to be able to synchronize on the shared object 
    722      
    723     public SynchronizedLineByLineHandler(BufferedOutputStream bos, int src) { 
     722    public SynchronizedLineByLineHandler(int src) { 
    724723        super(src); // will set this.source to STDERR or STDOUT 
    725         this.bos = bos; // caller will close bw, since many more than one 
    726                             // SynchronizedLineByLineHandlers are using it 
    727724    } 
    728725 
     
    744741 
    745742    // every time we read a line from the SafeProcess' stderr or stdout stream we come here. 
    746     // a different isntance of SynchronizedL 
    747     public synchronized void gotLine(String line) { 
    748         fireMessage(GShell.this.type,   // not final, needs synchro 
    749             GShell.this.typeAsString(type) + "> " + line, 
    750             GShell.this.status, // not final, needs synchro 
    751             this.bos);          // needs synchro 
     743    // Don't synchronize the gotLine() method, as it will synchronize on *this* (this object, this  
     744    // instance of LineByLineHandler). But we want to synchronize on the (outer class) GShell's 
     745    // variables that are used in fireMessage(). So it's the fireMessage(String) method that 
     746    // needs to be synchronized. See 
     747    // http://stackoverflow.com/questions/574240/is-there-an-advantage-to-use-a-synchronized-method-instead-of-a-synchronized-blo 
     748    public void gotLine(String line) { 
     749        fireMessage(line);     // synchronized, so even though process STDERR and STDOUT threads 
     750                               // will be firing messages, they won't be able to do so simultaneously 
    752751    } 
    753752