qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kenneth Giusti" <kgiu...@apache.org>
Subject Re: Review Request 22024: QPID-5790: avoid use of poll if greenthreads has monkey-patched select
Date Fri, 30 May 2014 13:26:17 GMT


> On May 30, 2014, 1:07 p.m., Rafael Schloming wrote:
> > /trunk/qpid/python/qpid/compat.py, line 50
> > <https://reviews.apache.org/r/22024/diff/1/?file=598726#file598726line50>
> >
> >     Isn't this check dependent on the order of importing with respect to monkey
patching? What happens if the monkey patching happens after compat.py gets imported?
> >     
> >     Given that in general it can be difficult to control the order in which imports
happen, I suspect this could appear to mysteriously break if compat.py happens to get imported
sooner rather than later. I would either do this check lazily, which unfortunately adds check
overhead everytime select is invoked, or alternatively add a mechanism for explicitly controlling
which implementation is chosen.

You are 100% correct - however I suspect the effort/complexity not worth it in light of the
eventlet documentation for monkey-patching: (see http://eventlet.net/doc/patching.html#monkey-patch)

Specifically from that page:

"It is important to call monkey_patch() as early in the lifetime of the application as possible.
Try to do it as one of the first lines in the main module. The reason for this is that sometimes
there is a class that inherits from a class that needs to be greened – e.g. a class that
inherits from socket.socket – and inheritance is done at import time, so therefore the monkeypatching
should happen before the derived class is defined. It’s safe to call monkey_patch multiple
times."

So I believe that the "correct" use case will be to monkey patch -before- importing qpid.

In general, this whole 'monkey-patching' thing gives me the willies.  I don't want to imply
anything other than the recommended use case is going to work (eg. late monkey-patching won't
be supported).


- Kenneth


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22024/#review44359
-----------------------------------------------------------


On May 30, 2014, 12:47 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22024/
> -----------------------------------------------------------
> 
> (Updated May 30, 2014, 12:47 p.m.)
> 
> 
> Review request for qpid, Gordon Sim, Rafael Schloming, and Ted Ross.
> 
> 
> Bugs: qpid-5790
>     https://issues.apache.org/jira/browse/qpid-5790
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Modify the python client to use select.select() instead of select.poll() if the application
has been monkey-patched by Eventlet/Greenthreads.
> 
> Eventlet only properly patches select.select(), not select.poll().  If select.poll()
is used, the application may hang.  This patch detects if the application is using a patched
select, and uses that instead of poll.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/python/qpid/compat.py 1598260 
> 
> Diff: https://reviews.apache.org/r/22024/diff/
> 
> 
> Testing
> -------
> 
> Tested the python client:
> 
> 1) running it in an environment that did not have eventlet installed - poll used
> 2) eventlet was installed, but select was not patched - poll used
> 3) select was patched - patched select was used.
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message