Tim Ellison wrote:
I realize that I'm talking to myself, but hey... :-)
Tim Ellison wrote:
However, close() also has the side effect of closing the source 'in'
stream, so we have to be prepared for that to be null at any
(unsynchronized) point too. The 'in' variable needs to be declared
volatile in FilterInputStream (as per spec too) -- but the problem is
that, unlike buf, simply holding a local reference to the stream doesn't
help if somebody changes it's state to closed since we can't continue to
rely on skip() and available() etc. working.
This is messy. I don't see how we can get a consistent state on 'in'?
On further thought, the local reference to the 'in' stream can only be
invalidated by the stream being closed, in which case our attempt to
call available() or read() should [1] simply result in an IOException
which is what we want anyway.
I've uploaded a patch [2] that enables the close() to be unsynchronized.
There is still one race condition in there, when we are replacing the
buf with a longer byte array. It may be set to null as we are replacing
it, and we would be left in an inconsistent state.
The new code is quite gross -- I really wonder what the use case is that
means close() should be unsynchronized??
One thread block on read, another one want to close the stream
immediately, if read and close are both synchronized, the second one
would be blocked until the first one read something.
I think, here the question is why we synchronized the whole read method.
Holding one lock and blocking on IO is dangerous, especially holding
"this" lock. Could we use private lock for synchronizing buffer?
[1] it could be subclassed to do something weird, but let's assume not.
[2] https://issues.apache.org/jira/browse/HARMONY-6014
Regards,
Tim
