Re: Note on PushbackInputStream

[Dalibor Topic]

Am Montag, 20. August 2001 17:51 schrieb Tom Tromey:
> >>>>> "Dalibor" == Dalibor Topic <address@hidden> writes:
> >>
> >> I can't see a way out of the situation either, without creating some
> >> kind of dual block (block on *either* the pushback or the underlying
> >> stream) or possibly setting a time interval to periodically come back
> >> from the underlying stream and check the pushback.
>
> Dalibor> I came up with the following pseudo-code:
> Dalibor> [ ... ]
>
> Dalibor> Thread A tries to read a single byte, and blocks. it keeps
> Dalibor> acquiring and releasing the lock for the pushbackinputstream
> Dalibor> while it blocks.
>
> I think polling like your loop does is a bad idea.  I would be very
> reluctant to put code like that into the library.

I completely understand that. It's a cycle-burner. It's not elegant. It was 
more of a proof-of-concept that you could fix the bug, without the need for 
internal threads etc. I have a more elegant solution using wait and notify 
which is quite close to the solution proposed by John. I wrote a simple test 
class WaitingPushbackInputsStream which extends PushbackInputStream. It has 
the following methods:

public class WaitingPushbackInputStream extends PushbackInputStream {
[...]
    public int read() throws IOException {
        synchronized(this) {
            synchronized(in) {
                while(0 == available()) {
                    try {                       
                        wait();
                    }
                    catch (InterruptedException e) {
                        // ignore
                    }
                }

                // something is available for reading,
                // and we've got both locks. this read 
                // should not block at all.
                return super.read();
            }
        }
    }

    public void unread(int oneByte) throws IOException {
        synchronized(this) {
            notifyAll();
            super.unread(oneByte);
        }
    }
}

this solution avoids polling, and only reads when there is something 
available for reading. It relies on being notified that there is something 
available. From what I've seen in my test, it doesn't burn cycles while it's 
waiting. 

I used notifyAll(), but in this case where you have a single byte to be 
(un)read, notify() should be just fine. I think that you need notifyAll() if 
you push back several bytes, since the notified thread might not want to read 
all bytes you pushed back. Then you'd be left with some threads waiting 
although there are bytes available in the pushback buffer.

The call to available() should take care of someone closing the 
PushbackInputStream or the sub-stream while a thread is waiting to read.

> I don't think this is actually a very important bug in practice.  I'm
> sure Sun's implementation has it too.  I just thought it was
> interesting from a design point of view.

I wrote a test, and both Blackdown's 1.1.8 and IBM's 1.3.0 have the bug. I 
think they share the class libraries with Sun, so I'll file a bug report 
later today.

> Dalibor> If you've got this far, I'd like to hear comments :-) I think
> Dalibor> that in a cooperative multi-threading environment the yield()
> Dalibor> is necessary, since otherwise thread B might not be able to
> Dalibor> get any CPU time to unread the byte, but I'm not sure.
>
> Thread.yield() isn't guaranteed to do anything.

That's right, but isn't it considered as "good practice" in 
continuous/polling loops ? That's how I interpret what Chang et al. say in  
"Java Class Libraries Second Edition Vol. 1" regarding Thread.yield().

thanks for the reply,

Dalibor Topic

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com