[erlang-bugs] Revert commit 6189bc07

chris jugg@REDACTED
Fri Feb 7 18:47:20 CET 2014


Hello,

The commit 6189bc07 "inets: httpc improve pipelining" needs to be reverted.

It introduces a crashing bug into the http client by inserting a session prior to a connection even being established, which will cause subsequent requests to crash that attempt to reuse that session.

In httpc_manager:start_handler/2, the session must not be inserted at that point, as there may not yet, nor ever be, a valid connection.

In the httpc_handler:handle_response(#state{status = new} = State) call chain from where the original call to insert_session was removed from httpc_handler:try_to_enable_pipeline_or_keep_alive, is actually where the session should be inserted.

>From the test case modification in that commit, it looks like the motivation was to get rid of a sleep(4000).  But out of the entire commit, what actually caused the ability to remove the sleep from the test, was simply that every session was now marked as 'available' (renamed 'persistent' in that commit) as soon as the request received a response, if the request established a persistent connection.

In other words, a one line code change to the original httpc_handler:try_to_enable_pipeline_or_keep_alive (renamed to httpc_handler:check_persistent and broken) could have had the same result - by always setting the 'available' session field to true for persistent connections.  Which would end up being for all stored sessions, as only persistent connections should have stored sessions anyway.  So given that, I question whether the result of such a modifictation is actually the proper thing to do, or whether the original code had its reasons for not making all persistent connections available until later in their life cycle.

In general the entire commit was ill-conceived, and introduces other subtle bugs as well.

Please revert the commit, as previously functional production code running for years against R16B02 and earlier releases is now crashing under R16B03(-1).

Regards,

Chris

ps. appologies if this message comes through twice, the first attempt through gmane seems to not have made it. 		 	   		  


More information about the erlang-bugs mailing list