classification
Title: selectors.SelectSelectors fails if select.select was patched
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: Przemyslaw Wegrzyn, brett.cannon, vstinner
Priority: normal Keywords: patch

Created on 2016-07-27 11:54 by Przemyslaw Wegrzyn, last changed 2016-07-27 16:48 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
t_s.py Przemyslaw Wegrzyn, 2016-07-27 11:54
selectors.diff Przemyslaw Wegrzyn, 2016-07-27 11:58 review
Messages (5)
msg271446 - (view) Author: Przemyslaw Wegrzyn (Przemyslaw Wegrzyn) Date: 2016-07-27 11:54
The SelectSelector makes a local copy of select.select() built-in and calls it via self._select later on. It no longer works if select.select() built-in is replaced with function (something gevent's monkey patching does).

Currently gevent employs a workaround - it overwrites SelectSelector._select. It wouldn't be necessary if SelectSelector could cope with select.select() being a regular function.

Attached is a file reproducing the issue.
msg271447 - (view) Author: Przemyslaw Wegrzyn (Przemyslaw Wegrzyn) Date: 2016-07-27 11:58
Possible workaround in the patch attached.
msg271452 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-07-27 14:04
Monkey-patching is a bad programming practice. I don't think that Python should promote this...

*But* selectors.diff LGTM.

I will wait one or two weeks to let others review the patch and give their opinion.
msg271462 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-07-27 16:48
I agree with Victor: monkeypatching is the wrong way to deal with this. Since SelectSelector is a class you are better off exposing something in the constructor or some method that can be overridden if this kind of flexibility is really necessary.
msg271463 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-07-27 16:48
Thanks for the patch, Przemyslaw, but I'm going to close this.
History
Date User Action Args
2017-01-21 04:15:01berker.peksaglinkissue29332 superseder
2016-07-27 16:48:50brett.cannonsetstatus: open -> closed
resolution: rejected
messages: + msg271463
2016-07-27 16:48:04brett.cannonsetnosy: + brett.cannon
messages: + msg271462
2016-07-27 14:04:53vstinnersetnosy: + vstinner
messages: + msg271452
2016-07-27 11:58:54Przemyslaw Wegrzynsetfiles: + selectors.diff
keywords: + patch
messages: + msg271447
2016-07-27 11:54:55Przemyslaw Wegrzyncreate