On Fri, Sep 17, 2010 at 7:55 AM, Amila Suriarachchi
<[email protected]> wrote:
>
>
>>
>> See https://issues.apache.org/jira/browse/AXIS2-4751 for details. The
>> thread safety problem was solved by not reusing HttpClient instance.
>
> As I told, this bug is reported with Axis2 1.5.1 not with trunk. hence this
> issue is not relevant.
>
> this is the original code.
> HttpClient httpClient;
>         Object reuse =
> msgContext.getOptions().getProperty(HTTPConstants.REUSE_HTTP_CLIENT);
>         if (reuse == null) {
>             reuse =
> msgContext.getConfigurationContext().getProperty(HTTPConstants.REUSE_HTTP_CLIENT);
>         }
>
>         if (reuse != null && JavaUtils.isTrueExplicitly(reuse)) {
>             httpClient = (HttpClient)
> msgContext.getOptions().getProperty(HTTPConstants.CACHED_HTTP_CLIENT);
>             if (httpClient == null) {
>                 httpClient = (HttpClient)
> msgContext.getConfigurationContext()
>                         .getProperty(HTTPConstants.CACHED_HTTP_CLIENT);
>             }
>             if (httpClient != null)
>                 return httpClient;
>             MultiThreadedHttpConnectionManager connectionManager =
>                     new MultiThreadedHttpConnectionManager();
>             httpClient = new HttpClient(connectionManager);
>             msgContext.getConfigurationContext()
>                     .setProperty(HTTPConstants.CACHED_HTTP_CLIENT,
> httpClient);
>         } else {
>             HttpConnectionManager connManager =
>                     (HttpConnectionManager) msgContext.getProperty(
>
> HTTPConstants.MULTITHREAD_HTTP_CONNECTION_MANAGER);
>             if (connManager == null) {
>                 connManager =
>                         (HttpConnectionManager) msgContext.getProperty(
>
> HTTPConstants.MUTTITHREAD_HTTP_CONNECTION_MANAGER);
>             }
>             if (connManager != null) {
>                 httpClient = new HttpClient(connManager);
>             } else {
>                 //Multi threaded http connection manager has set as the
> default
>                 connManager = new MultiThreadedHttpConnectionManager();
>                 httpClient = new HttpClient(connManager);
>             }
>         }
>
>         // Get the timeout values set in the runtime
>         initializeTimeouts(msgContext, httpClient);
>
>         return httpClient;
>
> it reuses http client only if user has set so. otherwise if the user has set
> an http connection manager object then
> it creates a new httpClient per every call.

This code is bad for two reasons:

1) In the first block when REUSE_HTTP_CLIENT is true, the code will
reuse HttpClient instance. As I mentioned before, that way we use
HttpClient instance in the code right now is not thread safe.

This directly relates to the key problem my original changes were addressing.

2) When REUSE_HTTP_CLIENT is not set or false and
MULTITHREAD_HTTP_CONNECTION_MANAGER is not set, the code will create a
new instance of MultiThreadedHttpConnectionManager and HttpClient.
Creating a new instance of MultiThreadedHttpConnectionManager on each
call means it is creating a new connection pool per call but the
connection pool is not really being reused. That can lead to a lot of
connections being open and remain open longer then needed.

An attempt was made in the 1.5 branch to ensure connection pool is
reused between the calls but that reused HttpClient instance instead
of HttpConnectionManager instance... and you know the rest.

So, please do not revert back to this version of the code.

Jarek

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to