Changeset 31667


Ignore:
Timestamp:
05/09/17 19:15:48 (4 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 edited

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
Note: See TracChangeset for help on using the changeset viewer.