You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@commons.apache.org by Attila Szegedi <sz...@freemail.hu> on 2004/05/28 19:44:23 UTC

[HttpClient] Clarification of documentation regarding connection release

Hi all,

the docs at <http://jakarta.apache.org/commons/httpclient/threading.html> state 
that this is the recommended way of using HttpClient to ensure connections are 
released:

try {
    client.executeMethod(get);
    ...
    } finally {
        get.releaseConnection();
    }

Now, this is counterintuitive to me. I'd expect the usage to be:

client.executeMethod(get);
try {
    ...
    } finally {
        get.releaseConnection();
    }

since it's the contract of all well-behaved libraries I came across to date that 
methods that exit abruptly (i.e. throw an exception) must not leave any 
resources allocated. Therefore, it must be safe to call executeMethod() 
*outside* the try block, since if it exits with an exception, the connection 
will not be allocated. I took the time to actually go through the source code of 
HttpClient 2.0, and have found out that:
- HttpClient.java lines 650-678 make sure that the connection object gets 
released if the connection can't be established. 
- HttpMethodBase.java lines 2663-2696 make sure that the connection is closed if 
an exception is encountered during sending request/receiving response. 
These catch blocks have the "doneWithConnection = true" statement that'll in 
turn cause releasing of the connection object in HttpMethodBase.java, 
lines 1122-1124.
 
The only case an already allocated connection object won't get released in a 
failed call to executeMethod() is in two cases of incorrect usage by the 
library's client. These are:
- a HttpMethod object is reused without being recycle()d first, (exception 
thrown in HttpMethodBase.java line 1007).
- preemptive authentication is used, but the supplied credentials aren't 
username/password but some other kind of credentials (exception thrown in 
HttpAuthenticator.java line 204).

(Now, you could claim that these conditions are incorrect usage of the library 
and that you feel no obligation to release the connection under these 
circumstances, but IMHO you should.)

Anyway, it'd be good if the documentation at the URL cited above would be 
changed to indicate that the executeMethod() should be called *outside* the try 
block - I think it's just the standard rule for all well-behaved libraries that 
users of the library are expected to release a resource only after the method 
that allocated the resource completes normally.

Regards,
  Attila.

-- 
home: http://www.szegedi.org
Visit Szegedi Butterfly fractals at:
  http://www.szegedi.org/fractals/butterfly/index.html



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Re: [HttpClient] Cookie name may not contain blanks

Posted by Oleg Kalnichevski <ol...@bluewin.ch>.
John,

I'll look into the problem and see how this restriction on cookie name
can be relaxed for the browser compatibility cookie policy

Oleg

On Mon, 2004-05-31 at 17:15, John Patterson wrote:
> Hi,
> 
> The Cookie class does not like names with spaces in them.  It throws an
> IllegalArgumentException.  Unfortunately the server that my app interacts
> with uses a space in the cookie name.  Both IE and Mozilla don't mind.
> Maybe this logic would be better placed in a CookieSpec class.
> 
> Thanks,
> 
> John.
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-user-help@jakarta.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


[HttpClient] Post may not be redirected

Posted by John Patterson <de...@hotmail.com>.
When I try to set PostMethod.setFollowRedirects(true) an exception is thrown
stating that this is not allowed with "Entity Enclosing requests".  However,
the server that I access does return a 302 for POST requests.  Again, both
IE and Mozilla allow this so it might be a good idea to change this
behaviour.  Perhaps it could be a configurable HttpMethodParam.

John.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


[HttpClient] Cookie name may not contain blanks

Posted by John Patterson <de...@hotmail.com>.
Hi,

The Cookie class does not like names with spaces in them.  It throws an
IllegalArgumentException.  Unfortunately the server that my app interacts
with uses a space in the cookie name.  Both IE and Mozilla don't mind.
Maybe this logic would be better placed in a CookieSpec class.

Thanks,

John.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Re: [HttpClient] Clarification of documentation regarding connection release

Posted by Attila Szegedi <sz...@freemail.hu>.
Oleg Kalnichevski <olegk <at> bluewin.ch> writes:

> 
> > You got me intrigued. I'll take a look at 3.0. Maybe in 3.0 the developers 
can 
> > reach a point of self-confidence sufficient to change the usage 
recommendation 
> > in the docs?
> > 
> 
> By the time HttpClient 3.0 goes BETA we will certainly revisit this
> issue. 
> 

Cool.

> Obviously you command a pretty good knowledge of HttpClient 2.0
> internals. 

LOL. In reality, all I know about it is a result of staring at the source code 
for 2-3 hours after my code using HttpClient was reviewed and the reviewer noted 
that it is buggy according to HttpClient docs since I wasn't putting the 
executeMethod() call inside a try block, but rather in front of it :-) 
Anyway, the source was readable enough to find my way around it rather quickly.

Since I could prove that 2.0 code won't actually leak connections even if the 
method call isn't in the guarded block, and since you clearly stated that you 
won't make changes in the future releases that'd mandate it to be inside the 
guarded block, I'm rather satisfied - my code is both correct and futureproof 
as-is.

> If you had a look at 3.0aplha1 and let us know whether you
> see it as improvement compared to 2.0, it would be greatly appreciated

I'll try to give it few peeks when time allows. I got somewhat hooked :-)

Cheers,
  Attila.

> 
> Oleg
> 

-- 
home: http://www.szegedi.org
Visit Szegedi Butterfly fractals at:
  http://www.szegedi.org/fractals/butterfly/index.html


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Re: [HttpClient] Clarification of documentation regarding connection release

Posted by Oleg Kalnichevski <ol...@bluewin.ch>.
> Gosh, and here we are (my company that is) introducing it into a 
> mission-critical production system...
> 

I was certainly a bit too harsh on HttpClient. I just feel very bitter
for having spent over two years of my life trying to bend the
limitations of the 2.0 API in all sorts of creative ways instead of just
doing things right

I just wish that certain design decision were not made the ways they had
been made. If not for just one but fatal design mistake (which is that
monstrous HttpMethod interface) we would not need a complete rewrite in
order to be able to fix known design limitations

Actually I myself have a mission critical system with 250,000 users that
makes a heavy use of HttpClient 2.0rc2 and I am not losing my sleep at
night. HttpClient 2.0 has been serving well in many, many other
applications, as our users have been telling us. Since the final 2.0
release (mid February to date) we have not have a single serious bug
report. HttpClient 2.0 may not be pretty but it proved quite reliable


> You got me intrigued. I'll take a look at 3.0. Maybe in 3.0 the developers can 
> reach a point of self-confidence sufficient to change the usage recommendation 
> in the docs?
> 

By the time HttpClient 3.0 goes BETA we will certainly revisit this
issue. 

Obviously you command a pretty good knowledge of HttpClient 2.0
internals. If you had a look at 3.0aplha1 and let us know whether you
see it as improvement compared to 2.0, it would be greatly appreciated

Oleg

> > 
> > (2) Connection leaks can and usually do cause a lot of grief. It's by
> > far better to be safe than sorry. I am kind of hesitant to change our
> > recommendation regarding connection release simply because HttpClient
> > does not feel like a well-behaved library. 
> 
> Well, for me it's a question of self-confidence and professionalism. I mean, I 
> know that I'm able to guarantee that any code I write doesn't leak resources on 
> exception. Actually, after I analysed the source code of HttpClient I claim that 
> authors of HttpClient 2.0 were also up to the task, and spent effort to ensure 
> that HttpClient#executeMethod *will not* leak connections - except for the two 
> cases of incorrect usage of the library. All it'd take is a single bold step (I 
> must admit "bold" here sounds a bit ironic to me as I write it) and update the 
> docs :-) Well, if not earlier, maybe in 3.0 you could vouch for the safety of 
> that code by updating the docs.
> 
> Mind you, I'm all for defensive programming, but I love to have behavioral 
> guarantees first, and only be defensive where the guarantees are lacking. Having 
> more guarantees from libraries helps keep my code cleaner :-) (i.e. I can't get 
> into a situation where I have to explain to a casual reviewer why is (or isn't) 
> that statement inside the try block).
> 
> > 
> > I does not make sense to pretend that HttpClient is a state-of-the-art
> > stuff because it is not. For the time being, we just want to make sure
> > that it is useful. HttpClient 4.0 (which is planned to be released as
> > Jakarta HttpClient) will have a fundamentally different (hopefully
> > better) architecture and different connection management. We'll do our
> > best to make sure that HttpClient is not just useful but also feels like
> > a well-behaved library
> 
> I appreciate that effort. However, 4.0 sounds too remote at the moment :-) We 
> have tasks at hand to solve here and for the time being we'll be sticking with 
> the 2.0. I just didn't want to have my executeMethod calls within try blocks. 
> Hurts aesthetically...
> 
> Thanks for your response,
>   Attila.
> 
> > 
> > Oleg
> > 
> > 
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-user-help@jakarta.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Re: [HttpClient] Clarification of documentation regarding connection release

Posted by Attila Szegedi <sz...@freemail.hu>.
Oleg Kalnichevski <olegk <at> bluewin.ch> writes:

> 
> Attila,
> 
> Your observation is certainly correct. HttpClient#executeMethod should
> never leave connection unreleased in case of abnormal termination.
> Unfortunately there are several reasons why I personally feel compelled
> to leave things as they stand
> 
> (1) HttpClient 2.0 API is fundamentally flawed in many ways. 

Gosh, and here we are (my company that is) introducing it into a 
mission-critical production system...

> For
> instance, the entire concept of method recycling is fundamentally wrong.
> There's virtually nothing to be gained from recycling method and our
> recommendation is NOT use HttpMethod#recycle at all

I can understand that - we're not recycling methods either. The two cases I 
discovered where HttpClient 2.0 can leave a dangling connection upon exception 
(method object reuse w/o recycling and preemptive auth with 
non-username-password credentials) can never occur in our code, fortunately. 
These are actually buggy usages of the library...

> 
> HttpClient 3.0 (which is now in the early ALPHA state) will address some
> of them, but not all. Several of existing design limitations require a
> complete rewrite. Basically HttpClient 2.0 is not worth the trouble. I
> believe HttpClient 3.0 should already have solved the two problem cases.

You got me intrigued. I'll take a look at 3.0. Maybe in 3.0 the developers can 
reach a point of self-confidence sufficient to change the usage recommendation 
in the docs?

> 
> (2) Connection leaks can and usually do cause a lot of grief. It's by
> far better to be safe than sorry. I am kind of hesitant to change our
> recommendation regarding connection release simply because HttpClient
> does not feel like a well-behaved library. 

Well, for me it's a question of self-confidence and professionalism. I mean, I 
know that I'm able to guarantee that any code I write doesn't leak resources on 
exception. Actually, after I analysed the source code of HttpClient I claim that 
authors of HttpClient 2.0 were also up to the task, and spent effort to ensure 
that HttpClient#executeMethod *will not* leak connections - except for the two 
cases of incorrect usage of the library. All it'd take is a single bold step (I 
must admit "bold" here sounds a bit ironic to me as I write it) and update the 
docs :-) Well, if not earlier, maybe in 3.0 you could vouch for the safety of 
that code by updating the docs.

Mind you, I'm all for defensive programming, but I love to have behavioral 
guarantees first, and only be defensive where the guarantees are lacking. Having 
more guarantees from libraries helps keep my code cleaner :-) (i.e. I can't get 
into a situation where I have to explain to a casual reviewer why is (or isn't) 
that statement inside the try block).

> 
> I does not make sense to pretend that HttpClient is a state-of-the-art
> stuff because it is not. For the time being, we just want to make sure
> that it is useful. HttpClient 4.0 (which is planned to be released as
> Jakarta HttpClient) will have a fundamentally different (hopefully
> better) architecture and different connection management. We'll do our
> best to make sure that HttpClient is not just useful but also feels like
> a well-behaved library

I appreciate that effort. However, 4.0 sounds too remote at the moment :-) We 
have tasks at hand to solve here and for the time being we'll be sticking with 
the 2.0. I just didn't want to have my executeMethod calls within try blocks. 
Hurts aesthetically...

Thanks for your response,
  Attila.

> 
> Oleg
> 
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Re: [HttpClient] Clarification of documentation regarding connection release

Posted by Oleg Kalnichevski <ol...@bluewin.ch>.
Attila,

Your observation is certainly correct. HttpClient#executeMethod should
never leave connection unreleased in case of abnormal termination.
Unfortunately there are several reasons why I personally feel compelled
to leave things as they stand

(1) HttpClient 2.0 API is fundamentally flawed in many ways. For
instance, the entire concept of method recycling is fundamentally wrong.
There's virtually nothing to be gained from recycling method and our
recommendation is NOT use HttpMethod#recycle at all

HttpClient 3.0 (which is now in the early ALPHA state) will address some
of them, but not all. Several of existing design limitations require a
complete rewrite. Basically HttpClient 2.0 is not worth the trouble. I
believe HttpClient 3.0 should already have solved the two problem cases.

(2) Connection leaks can and usually do cause a lot of grief. It's by
far better to be safe than sorry. I am kind of hesitant to change our
recommendation regarding connection release simply because HttpClient
does not feel like a well-behaved library. 

I does not make sense to pretend that HttpClient is a state-of-the-art
stuff because it is not. For the time being, we just want to make sure
that it is useful. HttpClient 4.0 (which is planned to be released as
Jakarta HttpClient) will have a fundamentally different (hopefully
better) architecture and different connection management. We'll do our
best to make sure that HttpClient is not just useful but also feels like
a well-behaved library

Oleg




On Fri, 2004-05-28 at 19:44, Attila Szegedi wrote:
> Hi all,
> 
> the docs at <http://jakarta.apache.org/commons/httpclient/threading.html> state 
> that this is the recommended way of using HttpClient to ensure connections are 
> released:
> 
> try {
>     client.executeMethod(get);
>     ...
>     } finally {
>         get.releaseConnection();
>     }
> 
> Now, this is counterintuitive to me. I'd expect the usage to be:
> 
> client.executeMethod(get);
> try {
>     ...
>     } finally {
>         get.releaseConnection();
>     }
> 
> since it's the contract of all well-behaved libraries I came across to date that 
> methods that exit abruptly (i.e. throw an exception) must not leave any 
> resources allocated. Therefore, it must be safe to call executeMethod() 
> *outside* the try block, since if it exits with an exception, the connection 
> will not be allocated. I took the time to actually go through the source code of 
> HttpClient 2.0, and have found out that:
> - HttpClient.java lines 650-678 make sure that the connection object gets 
> released if the connection can't be established. 
> - HttpMethodBase.java lines 2663-2696 make sure that the connection is closed if 
> an exception is encountered during sending request/receiving response. 
> These catch blocks have the "doneWithConnection = true" statement that'll in 
> turn cause releasing of the connection object in HttpMethodBase.java, 
> lines 1122-1124.
>  
> The only case an already allocated connection object won't get released in a 
> failed call to executeMethod() is in two cases of incorrect usage by the 
> library's client. These are:
> - a HttpMethod object is reused without being recycle()d first, (exception 
> thrown in HttpMethodBase.java line 1007).
> - preemptive authentication is used, but the supplied credentials aren't 
> username/password but some other kind of credentials (exception thrown in 
> HttpAuthenticator.java line 204).
> 
> (Now, you could claim that these conditions are incorrect usage of the library 
> and that you feel no obligation to release the connection under these 
> circumstances, but IMHO you should.)
> 
> Anyway, it'd be good if the documentation at the URL cited above would be 
> changed to indicate that the executeMethod() should be called *outside* the try 
> block - I think it's just the standard rule for all well-behaved libraries that 
> users of the library are expected to release a resource only after the method 
> that allocated the resource completes normally.
> 
> Regards,
>   Attila.


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org