race condition in test_keepalive_disconnected #84

Closed
teleological opened this Issue Mar 25, 2012 · 0 comments

Comments

Projects
None yet
2 participants

This test will sometimes fail due to a race condition in test code. The problem is local to the tests, and may not indicate a problem in the library code.

test_keepalive_disconnected(TestHTTPClient): ./httpclient/lib/httpclient/session.rb:858:in `parse_header': HTTPClient::KeepAliveDisconnected (HTTPClient::KeepAliveDisconnected)
    from ./lib/httpclient/session.rb:847:in `parse_header'
    from ./lib/httpclient/session.rb:830:in `read_header'
    from ./lib/httpclient/session.rb:652:in `get_header'
    from ./lib/httpclient.rb:1114:in `do_get_header'
    from ./lib/httpclient.rb:1063:in `do_get_block'
    from ./lib/httpclient.rb:869:in `do_request'
    from ./lib/httpclient.rb:961:in `protect_keep_alive_disconnected'
    from ./lib/httpclient.rb:868:in `do_request'

The problem occurs in the first assertion loop on lines 1313-1317. The array of threads created by this loop make requests which are handled by threads created by create_keepalive_disconnected_thread. Each of those handler threads respond successfully to a first request and then disconnect from a second request after reading the request line.

In order for the assertions to pass, each client thread must either:

  1. Obtain a new session, which will be handled successfully.
  2. Reuse a session from the session pool, which will be disconnected, and then obtain a new session which will be handled successfully.

However, since multiple threads are concurrently initiating and recycling sessions, it is possible for a thread to obtain two used threads, both of which are disconnected:

  1. Client thread A creates new session X, receives a successful response and returns the session to the pool.
  2. Client thread B obtains session X from the pool, receives a disconnect, and invalidates all sessions in the pool.
  3. Client thread C creates new session Y, receives a successful response and returns the session to the pool.
  4. Client thread B retries, obtaining session Y from the pool. It receives a disconnect and raises KeepAliveDisconnected.

If the intent of the invalidation at step 2 is to ensure that the request uses a new session, then there is also a race condition in the library.

I have only seen these failures happen with jruby (1.5.3 and 1.6.7), but I think that's due to the efficiency of jruby's thread scheduling, and that the problem could happen with other implementations.

I am attaching a patch that runs the problematic requests in series so that they are handled in a deterministic order. It doesn't look to me like this test verifies any behavior that is particular to concurrency, but please correct me if I've got that wrong.

@teleological teleological added a commit to teleological/httpclient that referenced this issue Mar 25, 2012

@teleological teleological Fix for issue #84: race condition in test 119d99f

nahi added the BUG label Nov 3, 2014

nahi closed this in c915058 Nov 17, 2014

@krukow krukow added a commit to calabash/calabash-android that referenced this issue Jan 9, 2015

@krukow krukow Update httpclient to 2.6.x
Fixes KeepAliveDisconnected races:

nahi/httpclient#84
nahi/httpclient#185
5d21473

krukow referenced this issue in calabash/calabash-android Jan 9, 2015

Merged

Update httpclient fix keepalivedisconnected #517

@TobiasRoikjer TobiasRoikjer pushed a commit to calabash/calabash-android-server that referenced this issue Jun 21, 2016

@krukow krukow Update httpclient to 2.6.x
Fixes KeepAliveDisconnected races:

nahi/httpclient#84
nahi/httpclient#185
f374e83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment