Changeset 31667

Show
Ignore:
Timestamp:
09.05.2017 19:15:48 (2 years ago)
Author:
ak19
Message:

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

Location:
main/trunk/greenstone3/src/java/org/greenstone
Files:
3 modified

Legend:

Unmodified
Added
Removed
  • main/trunk/greenstone3/src/java/org/greenstone/admin/guiext/Command.java

    r31665 r31667  
    201201 
    202202        // messageArea needs to be synchronized, since both the process' 
    203         // stderr and stdout will be attempting to append to it 
     203        // stderr and stdout will be attempting to append to it from their own threads 
    204204 
    205205        synchronized(_messageArea) { 
  • main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/CollectionConstructor.java

    r31578 r31667  
    121121    } 
    122122 
    123     protected void sendProcessStatus(ConstructionEvent evt) 
     123    protected synchronized void sendProcessStatus(ConstructionEvent evt) 
    124124    { 
    125125 
  • main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/GS2PerlConstructor.java

    r31631 r31667  
    738738        while ( (line = br.readLine()) != null ) { 
    739739             
    740             if(Thread.currentThread().isInterrupted()) { // should we not instead check if SafeProcess thread was interrupted? 
     740            // check if we got cancelled 
     741            if(Thread.currentThread().isInterrupted()) { // Did the SafeProcess thread interrupt us? Then break out of loop pre-maturely and finish up 
    741742            System.err.println("Got interrupted when reading lines from process err/out stream."); 
    742743            break; // will go to finally block 
     
    752753            //}  
    753754             
    754  
    755             this.gotLine(line); // synchronized 
    756              
    757             /* 
    758             try { 
    759             synchronized(bwHandle) { // get a lock on the writer handle, then write 
    760                  
    761                 bwHandle.write(line + "\n"); 
    762             }  
    763             } catch(IOException ioe) { 
    764             String msg = (this.source == SafeProcess.STDERR) ? "stderr" : "stdout"; 
    765             msg = "Exception when writing out a line read from perl process' " + msg + " stream."; 
    766             GS2PerlConstructor.logger.error(msg, ioe); 
    767             } 
    768              
    769             // this next method is thread safe since only synchronized methods are invoked. 
    770             // and only immutable (final) vars are used.  
    771             // NO, What about the listeners??? 
    772             sendProcessStatus(new ConstructionEvent(GS2PerlConstructor.this, GSStatus.CONTINUING, line));            
    773             */ 
     755            this.gotLine(line); // behaves threadsafe 
    774756        } 
    775757        } catch (IOException ioe) { // problem with reading in from process with BufferedReader br 
     
    780762        // now do what the original runPerlCommand() code always did: 
    781763        ioe.printStackTrace(); 
    782         logException(ioe); // synchronized 
     764        sendProcessStatus(new ConstructionEvent(this, GSStatus.ERROR, "Exception occurred " + ioe.toString())); // now synchronized 
    783765 
    784766        } catch (Exception e) { // problem with BufferedWriter bwHandle on processing each line 
    785767        e.printStackTrace(); 
    786         logException(e); // synchronized 
     768        sendProcessStatus(new ConstructionEvent(this, GSStatus.ERROR, "Exception occurred " + e.toString())); // now synchronized 
    787769        } finally { 
    788770        SafeProcess.closeResource(br); 
     
    790772    } 
    791773 
    792     // trying to keep synchronized methods as short as possible 
    793     private synchronized void logException(Exception e) { 
    794         sendProcessStatus(new ConstructionEvent(this, GSStatus.ERROR, "Exception occurred " + e.toString())); 
    795     } 
    796  
    797     // trying to keep synchronized methods as short as possible 
    798     private synchronized void gotLine(String line) throws Exception { 
     774    // helper method, deals with things in a thread-safe manner 
     775    private void gotLine(String line) throws Exception { 
    799776 
    800777        // BufferedWriter writes may not be atomic 
     
    802779        // Choosing to put try-catch outside of sync block, since it's okay to give up lock on exception 
    803780        // http://stackoverflow.com/questions/14944551/it-is-better-to-have-a-synchronized-block-inside-a-try-block-or-a-try-block-insi 
    804         try {                        
    805         bwHandle.write(line + "\n");     
     781        try { 
     782 
     783        // try to keep synchronized methods and synchronized blocks as short as possible 
     784        synchronized(bwHandle) { // get a lock on the writer handle, then write          
     785            bwHandle.write(line + "\n"); // can throw IOException 
     786        }  
    806787         
    807788///     GS2PerlConstructor.logger.info("@@@ WROTE LINE: " + line); 
    808789 
    809790        // this next method is thread safe since only synchronized methods are invoked. 
    810         // and only immutable (final) vars are used. 
     791        // and only immutable (final) vars are used. NO, What about the listeners??? 
     792        // So have made this next method synchronized, which synchronizes on the GS2PerlConstructor 
     793        // and consequently its member variables are protected against concurrent access 
     794        // by multiple threads. 
    811795        sendProcessStatus(new ConstructionEvent(GS2PerlConstructor.this, GSStatus.CONTINUING, line)); 
    812796         
    813         } catch(IOException ioe) { // can't throw Exceptions, but are forced to handle Exceptions here 
    814         // since our method definition doesn't specify a throws list. 
     797        } catch(IOException ioe) { 
    815798        // "All methods on Logger are multi-thread safe", see 
    816799        // http://stackoverflow.com/questions/14211629/java-util-logger-write-synchronization