You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Jonathan Burke <jb...@cs.washington.edu> on 2012/08/30 21:02:22 UTC
@ThreadSafe BasicClientConnectionManager is not threadsafe with
respect to shutdown
Hello,
Perhaps it's low priority since the comments of
BasicClientConnectionManager.java does say it ought to be handled by only
one thread but I would like to point out that it is not thread safe with
respect to shutdown in case you're interested.
The attached file includes a test I added to
httpclient/src/test/java/org/apache/http/impl/conn/TestBasicConnManager.java
that will demonstrate a null pointer exception raised by potentially
improper synchronization in BasicClientConnectionManager in relatively
short order.
Problem:
Since the assignment to variable shutdown in the shutdown method (line 262)
is not part of the synchronize block and none of the assertNotShutdown
method calls are within the synchronized blocks of their enclosing methods,
it is possible to have threads execute the commands of
BasicClientConnectionManager methods in the following sequence.
Thread 1
releaseConnection (or some other method that uses assertNotShutdown is
called)
assertNotShutdown - Passes - Line 183
Thread 2
shutdown
the shutdown flag becomes true - Line 262
shutdown releases this.poolEntry and this.conn in the synchronized(this)
block
Thread 1
release connection's synchronized(this) block get's executed
this.poolEntry is null in the try block causing a null pointer exception
- Line 211
this.poolEntry is null in the finally block causing a null pointer
exception - Line 224
Attached is a test case that will generate the attached stack-trace when
added to TestBasicConnManager and a possible fix (though the fix has a cost
of increased synchronization).
Thank you for your time.
Jonathan
Re: @ThreadSafe BasicClientConnectionManager is not threadsafe with
respect to shutdown
Posted by Jonathan Burke <jb...@cs.washington.edu>.
Absolutely.
On Thu, Aug 30, 2012 at 1:14 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Thu, 2012-08-30 at 12:02 -0700, Jonathan Burke wrote:
> > Hello,
> >
> >
> > Perhaps it's low priority since the comments of
> > BasicClientConnectionManager.java does say it ought to be handled by
> > only one thread but I would like to point out that it is not thread
> > safe with respect to shutdown in case you're interested.
> >
> >
> > The attached file includes a test I added to
> >
> httpclient/src/test/java/org/apache/http/impl/conn/TestBasicConnManager.java
> that will demonstrate a null pointer exception raised by potentially
> improper synchronization in BasicClientConnectionManager in relatively
> short order.
> >
> >
> > Problem:
> > Since the assignment to variable shutdown in the shutdown method (line
> > 262) is not part of the synchronize block and none of the
> > assertNotShutdown method calls are within the synchronized blocks of
> > their enclosing methods, it is possible to have threads execute the
> > commands of BasicClientConnectionManager methods in the following
> > sequence.
> >
> >
> > Thread 1
> > releaseConnection (or some other method that uses assertNotShutdown is
> > called)
> > assertNotShutdown - Passes - Line 183
> >
> >
> > Thread 2
> > shutdown
> > the shutdown flag becomes true - Line 262
> > shutdown releases this.poolEntry and this.conn in the
> > synchronized(this) block
> >
> > Thread 1
> > release connection's synchronized(this) block get's executed
> > this.poolEntry is null in the try block causing a null pointer
> > exception - Line 211
> > this.poolEntry is null in the finally block causing a null pointer
> > exception - Line 224
> >
> > Attached is a test case that will generate the attached stack-trace
> > when added to TestBasicConnManager and a possible fix (though the fix
> > has a cost of increased synchronization).
> >
> >
> > Thank you for your time.
> >
> >
> > Jonathan
>
> Jonathan
>
> Could you please raise a JIRA for this defect and attach the files to
> it?
>
> 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: @ThreadSafe BasicClientConnectionManager is not threadsafe with
respect to shutdown
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Thu, 2012-08-30 at 12:02 -0700, Jonathan Burke wrote:
> Hello,
>
>
> Perhaps it's low priority since the comments of
> BasicClientConnectionManager.java does say it ought to be handled by
> only one thread but I would like to point out that it is not thread
> safe with respect to shutdown in case you're interested.
>
>
> The attached file includes a test I added to
> httpclient/src/test/java/org/apache/http/impl/conn/TestBasicConnManager.java that will demonstrate a null pointer exception raised by potentially improper synchronization in BasicClientConnectionManager in relatively short order.
>
>
> Problem:
> Since the assignment to variable shutdown in the shutdown method (line
> 262) is not part of the synchronize block and none of the
> assertNotShutdown method calls are within the synchronized blocks of
> their enclosing methods, it is possible to have threads execute the
> commands of BasicClientConnectionManager methods in the following
> sequence.
>
>
> Thread 1
> releaseConnection (or some other method that uses assertNotShutdown is
> called)
> assertNotShutdown - Passes - Line 183
>
>
> Thread 2
> shutdown
> the shutdown flag becomes true - Line 262
> shutdown releases this.poolEntry and this.conn in the
> synchronized(this) block
>
> Thread 1
> release connection's synchronized(this) block get's executed
> this.poolEntry is null in the try block causing a null pointer
> exception - Line 211
> this.poolEntry is null in the finally block causing a null pointer
> exception - Line 224
>
> Attached is a test case that will generate the attached stack-trace
> when added to TestBasicConnManager and a possible fix (though the fix
> has a cost of increased synchronization).
>
>
> Thank you for your time.
>
>
> Jonathan
Jonathan
Could you please raise a JIRA for this defect and attach the files to
it?
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