Changeset 31666 for main

Timestamp:

2017-05-09T19:06:44+12:00 (7 years ago)

Author:

ak19

Message:

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

File:

• main/trunk/gli/src/org/greenstone/gatherer/shell/GShell.java (modified) (4 diffs)

main/trunk/gli/src/org/greenstone/gatherer/shell/GShell.java

@@ -357 +357 @@
     prcs = new SafeProcess(args);
     SafeProcess.LineByLineHandler processOutLineHandler
-        = new SynchronizedLineByLineHandler(bos, SafeProcess.STDOUT);
+        = new SynchronizedLineByLineHandler(SafeProcess.STDOUT);
     SafeProcess.LineByLineHandler processErrLineHandler
-        = new SynchronizedLineByLineHandler(bos, SafeProcess.STDERR);
+        = new SynchronizedLineByLineHandler(SafeProcess.STDERR);
     
     prcs.setExceptionHandler(this); 
@@ -524 +524 @@
     }
 
-
-    public void fireMessage(String message)
+    // Now synchronized, since separate threads handling process out and err streams can call fireMessage()
+    // and need to reserve member objects and member variables used in this method while doing so.
+    public synchronized void fireMessage(String message)
     {
     fireMessage(type, typeAsString(type) + "> " + message, status, buffered_output_stream);
@@ -719 +720 @@
     protected class SynchronizedLineByLineHandler extends SafeProcess.LineByLineHandler
     {
-    private final BufferedOutputStream bos; // needs to be final to be able to synchronize on the shared object
-    
-    public SynchronizedLineByLineHandler(BufferedOutputStream bos, int src) {
+    public SynchronizedLineByLineHandler(int src) {
         super(src); // will set this.source to STDERR or STDOUT
-        this.bos = bos; // caller will close bw, since many more than one
-                            // SynchronizedLineByLineHandlers are using it
     }
 
@@ -744 +741 @@
 
     // every time we read a line from the SafeProcess' stderr or stdout stream we come here.
-    // a different isntance of SynchronizedL
-    public synchronized void gotLine(String line) {
-        fireMessage(GShell.this.type,   // not final, needs synchro
-            GShell.this.typeAsString(type) + "> " + line,
-            GShell.this.status, // not final, needs synchro
-            this.bos);          // needs synchro
+    // Don't synchronize the gotLine() method, as it will synchronize on *this* (this object, this 
+    // instance of LineByLineHandler). But we want to synchronize on the (outer class) GShell's
+    // variables that are used in fireMessage(). So it's the fireMessage(String) method that
+    // needs to be synchronized. See
+    // http://stackoverflow.com/questions/574240/is-there-an-advantage-to-use-a-synchronized-method-instead-of-a-synchronized-blo
+    public void gotLine(String line) {
+        fireMessage(line);     // synchronized, so even though process STDERR and STDOUT threads
+                               // will be firing messages, they won't be able to do so simultaneously
     }