Re: java.io PipedInputStream PipedOutputStream PipedReader PipedWriter

[Bryce McKinlay <bryce at albatross dot co dot nz>]

Tom Tromey wrote:

> >>>>> "Bryce" == Bryce McKinlay <bryce@albatross.co.nz> writes:
>
> Bryce> The Piped I/O classes we inherited from classpath were somewhat
> Bryce> broken (especially PipedReader/PipedWriter). These new
> Bryce> implementations are much simpler and hopefully less buggy -
> Bryce> they work for my code and pass all the test cases I found (JCL,
> Bryce> Kaffe, Classpath).
>
> There are some Mauve tests too.
>
> BTW I noticed in the Reader classes you replaced synchronized(lock)
> with synchronization on the pipe itself.  This seems wrong.  I thought
> Readers were always supposed to synchronize on their lock object.

Good point. Thats because I did PipedReader/Writer by copying the code
from PipedInputStream/OutputStream and doing a few search & replaces on
it ;-)

I'm checking in this fix:


2001-01-06  Bryce McKinlay  <bryce@albatross.co.nz>

	* java/io/PipedReader: Synchronize on "lock" instead of this.

Index: PipedReader.java
===================================================================
RCS file: /cvs/gcc/egcs/libjava/java/io/PipedReader.java,v
retrieving revision 1.6
diff -u -b -r1.6 PipedReader.java
--- PipedReader.java	2001/01/05 08:23:51	1.6
+++ PipedReader.java	2001/01/05 23:27:26
@@ -141,9 +141,11 @@
     *           put it here in order to support that bizarre recieve(int)
     *           method.
     */  
-  synchronized void receive(char[] buf, int offset, int len)
+  void receive(char[] buf, int offset, int len)
     throws IOException
   {
+    synchronized (lock)
+    {
     if (closed)
       throw new IOException ("Pipe closed");
 
@@ -157,8 +159,8 @@
 	    while (in == out)
 	      {
 		// The pipe is full. Wake up any readers and wait for them.
-		notifyAll();
-		wait();
+		  lock.notifyAll();
+		  lock.wait();
 		// The pipe could have been closed while we were waiting.
 	        if (closed)
 		  throw new IOException ("Pipe closed");
@@ -188,7 +190,8 @@
 	  in = 0;
       }
     // Notify readers that new data is in the pipe.
-    notifyAll();
+      lock.notifyAll();
+    }
   }
   
   /**
@@ -240,9 +243,11 @@
     * @exception IOException If <code>close()/code> was called on this Piped
     *                        Reader.
     */  
-  public synchronized int read(char[] buf, int offset, int len)
+  public int read(char[] buf, int offset, int len)
     throws IOException
   {
+    synchronized (lock)
+    {
     if (source == null)
       throw new IOException ("Not connected");
     if (closed)
@@ -256,7 +261,7 @@
 	  {
 	    if (source.closed)
 	      return -1;
-	    wait();
+	      lock.wait();
 	  }
       }
     catch (InterruptedException ix)
@@ -297,17 +302,20 @@
 	  {
 	    // Notify any waiting Writer that there is now space
 	    // to write.
-	    notifyAll();
+	      lock.notifyAll();
 	    return total;
 	  }
       }
   }
+  }
   
-  public synchronized boolean ready() throws IOException
+  public boolean ready() throws IOException
   {
     // The JDK 1.3 implementation does not appear to check for the closed or 
     // unconnected stream conditions here.
     
+    synchronized (lock)
+    {
     if (in < 0)
       return false;
     
@@ -319,6 +327,7 @@
     
     return (count > 0);
   }
+  }
   
   /**
   * This methods closes the stream so that no more data can be read
@@ -326,10 +335,13 @@
   *
   * @exception IOException If an error occurs
   */
-  public synchronized void close() throws IOException
+  public void close() throws IOException
   {
+    synchronized (lock)
+    {
     closed = true;
     // Wake any thread which may be in receive() waiting to write data.
-    notifyAll();
+      lock.notifyAll();
+    }
   }
 }