Bug #3492

get_http_connection() patches .close() which gets called from httplib itself causing gevent AssertionErrors

Added by Georgios Tsoukalas about 9 years ago. Updated about 9 years ago.

Status:Closed Start date:03/22/2013
Priority:High Due date:
Assignee:Georgios Tsoukalas % Done:

0%

Category:pooling Spent time: -
Target version:0.13.0

Description

psomas used cstavr's method of reproducing the elusive AssertionErrors
and tracked the cause in a call to self.close() inside httplib itself.

1. get_http_connection() patches httplib's HTTPConnection's .close()
to a version that would just return the connection to the pool.

2. resp = conn.getresponse() calls self.close() which puts the
connection back in the pool.

3. body = resp.read() uses the now pooled connection

4. Another greenlet gets the connection from the pool and fails to use it
because sockets cannot be shared among greenlets

Notes:

  • httplib closes calls self.close() because sometimes the
    HTTP Protocol says it should (e.g. Connection: close header)
  • The choice to patch .close() instead of providing a distinct
    method .pool_release() was done so that no change would be made
    to the connection API.
    Clearly this was a wrong choice, not only because of this problem,
    but also since the API has been already severely breached since
    all pooled connection users are required to never let the connection
    objects fall out of scope without calling .close() within appropriate
    try statements.
  • Since our real pooling target is sockets, not httplib connections,
    it would be better if we pooled sockets instead.
    However, since we don't directly handle sockets we must rely
    on the libraries themselves for socket pooling (httplib3?)
  • It seems that renaming the patched method from .close()
    to another name, and using it as such will solve the problem.

Related issues

related to Synnefo - Bug #3414: AssertionError in Astakos gunicorn Closed 03/08/2013

Associated revisions

Revision 1bce7b1f
Added by Georgios D. Tsoukalas about 9 years ago

pool context manager, http pool AssertionError fix

Refs #3492

1 Introduce a generic PooledObject class to act both
as a context manager for getting and putting back
an object from a pool.

2 Implement a class PooledHTTPConnection(PooledObject)
as a sublcass of the one in (1)

3 Eliminate httplib.HTTPConnection patching of close().
Eliminate put/get_http_connection.
The httplib.HTTPConnection object is no longer released
back to the pool via a method on it.
One must explicitly put it to the pool,
or use the PooledObject context manager in (2) above.

4 Update lib.astakos, lib.quotaholder.http,
synnefo.api.delegate, pithos.api.delegate to use
PooledHTTPConnection.

5 Update tests

Revision 4ab1af1a
Added by Georgios D. Tsoukalas about 9 years ago

pool context manager, http pool AssertionError fix

Refs #3492

- Introduce a generic PooledObject class to act both
as a context manager for getting and putting back
an object from a pool.

- Implement a class PooledHTTPConnection(PooledObject)
as a sublcass of the one in (1)

- Eliminate httplib.HTTPConnection patching of close().
Eliminate put/get_http_connection.
The httplib.HTTPConnection object is no longer released
back to the pool via a method on it.
One must explicitly put it to the pool,
or use the PooledObject context manager in (2) above.

- Update lib.astakos, lib.quotaholder.http,
synnefo.api.delegate, pithos.api.delegate to use
PooledHTTPConnection.

- Update tests

History

#1 Updated by Georgios Tsoukalas about 9 years ago

  • Status changed from Assigned to Resolved

#2 Updated by Georgios Tsoukalas about 9 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF