[prev in list] [next in list] [prev in thread] [next in thread] 

List:       httpclient-commons-dev
Subject:    Re: svn commit: r767657 - /httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/
From:       sebb <sebbaz () gmail ! com>
Date:       2009-04-22 23:28:43
Message-ID: 25aac9fc0904221628o4c640edap3c325725b67e4d8e () mail ! gmail ! com
[Download RAW message or body]

On 22/04/2009, olegk@apache.org <olegk@apache.org> wrote:
> Author: olegk
> Date: Wed Apr 22 21:03:47 2009
> New Revision: 767657
> 
> URL: http://svn.apache.org/viewvc?rev=767657&view=rev
> Log:
> Made class thread-safe; cleaned up javadoc
> Modified:
> httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java
>  

I'm not convinced that the class is thread-safe.

uniquePoolEntry is assigned in the constructor, but is not final, so
is not necessarily published safely. Also revokeConnection() accesses
it but is not synchronized. I think this can be fixed by making the
field final and synching revokeConnection().

alwaysShutDown is not final, and is not volatile so the constructor
does not safely publish it. I think it could be made final.

Unless there are objections, I propose to make the above changes.

Most of the fields are protected rather than private; this makes it
possible for sub-classes to bypass thread-safety requirements. It
seems to me that fields (especially mutable ones) should default to
the minimum visibility possible, unless it is essential that they
accessible externally. There has yet to be a GA release, so now is the
time to lock things down before they "escape".

Making fields private will change the API, so I think that needs a bit
more discussion.

S///

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


[prev in list] [next in list] [prev in thread] [next in thread]