Shutting down the connection manager

[GoogleCodeExporter]

According to HttpClient documentation, "When an HttpClient instance is no 
longer needed and is about to go out of scope it is important to shutdown its 
connection manager" :
httpclient.getConnectionManager().shutdown();

However, it seems that the connection manager is not shutdown in SardineImpl.

Original issue reported on code.google.com by cchaban...@gmail.com on 26 Jan 2012 at 8:20

[GoogleCodeExporter]

The relevant portions of the documentation are: "and is about to go out of 
scope". We use the ThreadSafeClientConnManager by default, so repeated requests 
are handled by that. We don't really go out of scope during the execution of 
Sardine.

I'll close this in the next couple of days unless you update with more specific 
details of why you are reporting this as an issue.

Original comment by latch...@gmail.com on 26 Jan 2012 at 9:18

[GoogleCodeExporter]

When we don't use the Sardine object anymore, the connection manager (that 
Sardine created) should be shutdown to ensure immediate deallocation of all 
system resources or it will be shutdown in its finalize method which is not 
ideal.

Original comment by cchaban...@gmail.com on 26 Jan 2012 at 10:20

[GoogleCodeExporter]

Again, is this causing you a specific problem? Shutting down in finalize is 
fine, that is the point of finalize.

Original comment by latch...@gmail.com on 26 Jan 2012 at 6:25

[GoogleCodeExporter]

Really ?
finalize isn't guaranteed to be called, so you should not count on it releasing 
resources. You should not rely on finalizers to release non-memory resources.
http://www.javaworld.com/javaworld/jw-06-1998/jw-06-techniques.html

You should also see the samples at 
http://hc.apache.org/httpcomponents-client-ga/examples.html

You should definitely add a close() method on Sardine.

Original comment by cchaban...@gmail.com on 26 Jan 2012 at 7:57

[GoogleCodeExporter]

I've asked you two times to point out if this is actually affecting you. I 
haven't gotten a response other than to quote an article from 1998!?! 

Sardine doesn't have the concept of a lifecycle. Nor does it need to shut down. 
The JVM is either running or it is dead, therefore closing down HttpClient is a 
noop is this case. Reading the shutdown code, all that happens is that the pool 
is closed and any connections are closed. Effectively, the only thing that 
happens is that memory is released. Whatever, if the JVM is dead, that releases 
a lot of memory too! Because this is a client connection, the OS will deal just 
fine with cleaning up any potential open resources left open after the JVM has 
terminated.

Sardine has been doing just fine for many years now without this as an issue. 
If it bothers you so much, then feel free to fork the code and add this 
yourself. 

Sorry, I'm closing this as a WontFix.

Original comment by latch...@gmail.com on 26 Jan 2012 at 8:12

• Changed state: WontFix

[GoogleCodeExporter]

It does not affect me yet : I use Sardine only for unit tests for now.
Do you mean that I should use only one sardine instance for the whole JVM 
lifetime ?

Original comment by cchaban...@gmail.com on 26 Jan 2012 at 8:26

[GoogleCodeExporter]

My point is that it won't affect you.

You only need one instance of Sardine, however you need to pay attention to the 
way that threading works if you are running it multithreaded. There is no need 
to use multiple instances of Sardine, but note that the connection manager will 
block if you run out of configured threads. For unit tests, it is probably 
safest to just call begin() each time you want to use Sardine.

http://code.google.com/p/sardine/wiki/UsageGuide#Threading

Original comment by latch...@gmail.com on 26 Jan 2012 at 8:33

[GoogleCodeExporter]

You can also use your own client instance and connection manager configured to 
your needs by overriding the relevant methods or calling the constructor of the 
SardineImpl class with your own instance.

Original comment by dkocher@sudo.ch on 26 Jan 2012 at 9:23

[GoogleCodeExporter]

I am not sure this is a non issue, though. We should release resources that we 
allocate and possibly the garbage collector will not cleanup at all if threads 
from the connection pool are still alive. 

Original comment by dkocher@sudo.ch on 26 Jan 2012 at 9:30

[GoogleCodeExporter]

If Sardine is being used in an environment that needs to restart without 
restarting the JVM, then it *could* be an issue. In that case, then I'd suggest 
overriding the client. 

But other than that case, which is a *rare* case at best, I don't see this as a 
big issue since the JVM is shut down entirely.

This is an optimization for failure. ;-)

Original comment by latch...@gmail.com on 26 Jan 2012 at 9:39

[GoogleCodeExporter]

I think it should be discussed if it is a common use case that the JVM has a 
longer lifecycle than Sardine. This is legitimate and could well be the case 
for many users of the library.

Original comment by p...@iterate.ch on 26 Jan 2012 at 10:31

[GoogleCodeExporter]

All you have to do is implement your own SardineFactory, which instantiates a 
SardineImpl with your own AbstractHttpClient instance. Then, add a method to 
your SardineFactory which is you can call during shutdown and calls 
client...shutdown().

I wouldn't have put final on SardineFactory but for some reason David (I think) 
did. This would have at least given you a base for your own factory. That said, 
it is a trivial class.

I like that for 99% of the usecases out there, Sardine is a very simple 
interface. For the 1%, you can worry about shutting things down yourself.

Original comment by latch...@gmail.com on 26 Jan 2012 at 11:06