biasedbit/http-client

Connections are not closed and released when sockets throw exceptions

Closed this issue · 6 comments

When an exception is notified to DefaultHttpConnection, the exceptionCaught method invokes terminate, but terminate doesn't tell the listener that the connection is terminated, so it is never removed from the pool and when the maximum number of concurrent connections is reached, the (Abstract)HttpClient can't connect any more.
Perhaps the channelClosed method is supposed to do that, but it doesn't as it finds the this.terminate variable already set and quits soon.

My temporary fix is to call "this.listener.connectionTerminated(this);" at the very end of the terminate() method, but it would probably correct to deal with the channelClosed method or the currentRequest and its Future in a different way.

I had this happening all the time with the socket throwing a SocketException with a reasonMessage of "Connection reset".

Duplicate of #1

Bruno, I'm running the latest code from Jatinder Singh, which basically only has 1 difference with yours, in a bit of code that - thanks to added logging - I can tell is never run when we have that failure (and that is the actual problem!), so this is definitely not a duplicate of #1.

I'm running jdk1.7.0_13

My bad, terribly sorry. I haven't touched this project in a long time so I'm a little bit out of context, do you have a fix you want to propose via PR or want me to look into this?

No problem, I understand.

I have a quick and dirty fix, which is adding...
this.listener.connectionTerminated(this);
...at the very end of DefaultHttpConnection.terminate(Throwable reason), but I'm waiting a few days, maybe a week, to check that the problem has gone. That fix might be OK, by looking at what "channelClosed(ChannelHandlerContext ctx, ChannelStateEvent e)" does, but I haven't looked so much at the rest of the code and there might be other things to do.

That sounds good, lemme know how it works out.

TBH, I should start doing version 2.0 of this thing... Netty 4, specs, Java 7, lombok (to get rid of all those getters and setters). Or at least update to the latest 3.x version of Netty and spec the whole thing up to avoid these issues.