You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2009/06/04 20:07:42 UTC
svn commit: r781814 - in
/httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util:
SharedInputBuffer.java SharedOutputBuffer.java
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.
*
* @since 4.0
*/
Modified: httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedOutputBuffer.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedOutputBuffer.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/SharedOutputBuffer.java (original)
+++ httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedOutputBuffer.java Thu Jun 4 18:07:42 2009
@@ -39,7 +39,7 @@
/**
* Implementation of the {@link ContentOutputBuffer} 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.
*
* @since 4.0
*/
Re: svn commit: r781814
Posted by sebb <se...@gmail.com>.
On 06/06/2009, Oleg Kalnichevski <ol...@apache.org> wrote:
> sebb wrote:
>
> > On 05/06/2009, Oleg Kalnichevski <ol...@apache.org> wrote:
> >
> > > sebb wrote:
> > >
> > >
> > > > On 04/06/2009, olegk@apache.org <ol...@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.
Indeed, but the point here is that these unsynch. methods must not be
called if the interface methods are to remain thread-safe. So I
suggest to either override and throw an Exception, or override and
synch.
Well-behaved code will not be affected, because those superclass
methods which are not in the inetrface must not be called directly.
The advantage is that naughty code won't be able to cause problems.
> 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r781814
Posted by Oleg Kalnichevski <ol...@apache.org>.
sebb wrote:
> On 05/06/2009, Oleg Kalnichevski <ol...@apache.org> wrote:
>> sebb wrote:
>>
>>> On 04/06/2009, olegk@apache.org <ol...@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
Re: svn commit: r781814
Posted by sebb <se...@gmail.com>.
On 05/06/2009, Oleg Kalnichevski <ol...@apache.org> wrote:
> sebb wrote:
>
> > On 04/06/2009, olegk@apache.org <ol...@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().
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.
Unless there are good reasons for leaving the non-thread-safe methods
exposed, why not override them?
> 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
Re: svn commit: r781814
Posted by Oleg Kalnichevski <ol...@apache.org>.
sebb wrote:
> On 04/06/2009, olegk@apache.org <ol...@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.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r781814 - in /httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util:
SharedInputBuffer.java SharedOutputBuffer.java
Posted by sebb <se...@gmail.com>.
On 04/06/2009, olegk@apache.org <ol...@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?
It looks like the super-class (ExpandableBuffer) is not thread-safe -
for example the mode field is mutable and is not synchronized by the
available() and hasData() public methods, nor are these methods
overridden by SharedInputBuffer.
Similarly for SharedOutputBuffer below.
> * @since 4.0
> */
>
> Modified: httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedOutputBuffer.java
> URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedOutputBuffer.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/SharedOutputBuffer.java (original)
> +++ httpcomponents/httpcore/branches/4.0.x/httpcore-nio/src/main/java/org/apache/http/nio/util/SharedOutputBuffer.java Thu Jun 4 18:07:42 2009
> @@ -39,7 +39,7 @@
> /**
> * Implementation of the {@link ContentOutputBuffer} 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.
> *
> * @since 4.0
> */
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org