hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: svn commit: r781814
Date Sat, 06 Jun 2009 11:51:49 GMT
sebb wrote:
> On 05/06/2009, Oleg Kalnichevski <olegk@apache.org> wrote:
>> sebb wrote:
>>
>>> On 04/06/2009, olegk@apache.org <olegk@apache.org> wrote:
>>>
>>>> Author: olegk
>>>>  Date: Thu Jun  4 18:07:42 2009
>>>>  New Revision: 781814
>>>>
>>>>  URL: http://svn.apache.org/viewvc?rev=781814&view=rev
>>>>  Log:
>>>>  Javadoc fix
>>>>
>>>>  Modified:
>>>>
>> httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java
>> httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedOutputBuffer.java
>>>>  Modified:
>> httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java
>>>>  URL:
>> http://svn.apache.org/viewvc/httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java?rev=781814&r1=781813&r2=781814&view=diff
>> ==============================================================================
>>>>  ---
>> httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java
>> (original)
>>>>  +++
>> httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java
>> Thu Jun  4 18:07:42 2009
>>>>  @@ -39,7 +39,7 @@
>>>>  /**
>>>>  * Implementation of the {@link ContentInputBuffer} interface that can
>> be
>>>>  * shared by multiple threads, usually the I/O dispatch of an I/O
>> reactor and
>>>>  - * a worker tread. This class is not threading safe.
>>>>  + * a worker thread. This class is thread safe.
>>>>
>>> Are you sure it is thread-safe?
>>>
>>>
>>  The interface impl is. I rephrased the javadoc to reflect that. I hope it
>> is all right like that.
> 
> I think it's still not thread-safe, because consumeContent() calls
> setInputMode(). This is done in a synch. block, but another thread can
> call setInputMode() or setOutputMode() directly. AFAICS, this can
> cause problems for consumeContent().
> 

Sebastian,

Instances of those class are meant to be interacted with thought the 
interface. That is the whole point of having the classes implement an 
interface to start with. I wish ExpandableBuffer was never made public / 
had any public methods in the first place.

> One way to prevent this would be to override the setxxxxMode() methods
> with synch. versions or even with versions that throw an Exception.
> 
> The interface methods appear to be thread-safe, but that is currently
> only true if certain other methods are not use. If the code is to
> remain as it is, then I think the Javadoc needs to be much more
> explicit.
> 
> Something like:
> 
> This class is not thread-safe. However, the methods which implement
> ContentInputBuffer are thread-safe, provided that none of the other
> methods are called.
> 

I tweaked the javadocs once again. Please review and make changes you 
deem necessary.

> Unless there are good reasons for leaving the non-thread-safe methods
> exposed, why not override them?
> 

I would like to avoid over-synchronization as this class is performance 
  critical.

Oleg

>>  Oleg
>>
>>
>> ---------------------------------------------------------------------
>>  To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
>>  For additional commands, e-mail: dev-help@hc.apache.org
>>
>>
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message