harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Regis <xu.re...@gmail.com>
Subject Re: [classlib][luni] BufferedInputStream can not be closed in another thread (HARMONY-6014)
Date Fri, 14 Nov 2008 12:44:45 GMT


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
> 

Mime
View raw message