classification
Title: ProactorEventLoop.close(): set_wakeup_fd() only works in main thread
Type: crash Stage: resolved
Components: asyncio Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, carltongibson, felixxm, miss-islington, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2019-10-23 12:13 by carltongibson, last changed 2019-10-23 16:38 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 16901 merged vstinner, 2019-10-23 14:27
PR 16902 merged miss-islington, 2019-10-23 15:25
Messages (11)
msg355214 - (view) Author: Carlton Gibson (carltongibson) Date: 2019-10-23 12:13
6ea29c5e90dde6c240bd8e0815614b52ac307ea1 for 
https://bugs.python.org/issue34687

"Make asynico use ProactorEventLoop by default"

This introducesd a regression in asgiref, and hence Django 3.0.x. 


https://github.com/django/asgiref/issues/132
https://code.djangoproject.com/ticket/30900


The error from the asgiref issue is clearest: 

```
asgiref\sync.py:97: in __call__
    loop_future.result()
..\..\..\AppData\Local\Programs\Python\Python38\lib\concurrent\futures\_base.py:432: in result
    return self.__get_result()
..\..\..\AppData\Local\Programs\Python\Python38\lib\concurrent\futures\_base.py:388: in __get_result
    raise self._exception
..\..\..\AppData\Local\Programs\Python\Python38\lib\concurrent\futures\thread.py:57: in run
    result = self.fn(*self.args, **self.kwargs)
asgiref\sync.py:130: in _run_event_loop
    loop.close()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  

self = <ProactorEventLoop running=False closed=False debug=False>

    def close(self):
        if self.is_running():
            raise RuntimeError("Cannot close a running event loop")
        if self.is_closed():
            return
    
>       signal.set_wakeup_fd(-1)
E       ValueError: set_wakeup_fd only works in main thread
```

Off the main thread, we use a ThreadPoolExecutor and wait for the future to finish. 

It looks as if `ProactorEventLoop` needs to check the current thread before triggering the signal in close().
msg355216 - (view) Author: Carlton Gibson (carltongibson) Date: 2019-10-23 12:22
Reproduce steps, on Windows with Python 3.8.0, and a clean venv. 

git clone https://github.com/django/asgiref.git
cd asgiref
pip install -e .[tests]
py.test .
msg355217 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-10-23 13:12
Pull Request is welcome!
msg355220 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-23 14:28
That's basically the same issue than bpo-34679 but on close() rather than on creating the event loop.
msg355222 - (view) Author: Carlton Gibson (carltongibson) Date: 2019-10-23 14:43
Thanks for the speedy patch Victor. Great effort!
msg355224 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-23 14:46
> Thanks for the speedy patch Victor. Great effort!

Can you try to manually test PR 16901?
msg355225 - (view) Author: Carlton Gibson (carltongibson) Date: 2019-10-23 14:50
OK, that seems fair :) Will do that tomorrow AM.
msg355231 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-23 15:25
New changeset 1b53a24fb4417c764dd5933bce505f5c94249ca6 by Victor Stinner in branch 'master':
bpo-34679: ProactorEventLoop only uses set_wakeup_fd() in main thread (GH-16901)
https://github.com/python/cpython/commit/1b53a24fb4417c764dd5933bce505f5c94249ca6
msg355234 - (view) Author: miss-islington (miss-islington) Date: 2019-10-23 15:44
New changeset cbf474c98e702d12c97cd16a1e44ede10ea52b5b by Miss Skeleton (bot) in branch '3.8':
bpo-34679: ProactorEventLoop only uses set_wakeup_fd() in main thread (GH-16901)
https://github.com/python/cpython/commit/cbf474c98e702d12c97cd16a1e44ede10ea52b5b
msg355235 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-23 15:46
Yury and Andrew approved my PR 16901, so I merged it.

Thanks for the bug report Carlton Gibson.
msg355240 - (view) Author: Carlton Gibson (carltongibson) Date: 2019-10-23 16:32
Super turn-around. Thank you all.
History
Date User Action Args
2019-10-23 16:38:10vstinnersetstatus: open -> closed
resolution: fixed
2019-10-23 16:32:54carltongibsonsetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg355240
2019-10-23 15:46:00vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg355235

stage: patch review -> resolved
2019-10-23 15:44:03miss-islingtonsetnosy: + miss-islington
messages: + msg355234
2019-10-23 15:25:55miss-islingtonsetpull_requests: + pull_request16440
2019-10-23 15:25:38vstinnersetmessages: + msg355231
2019-10-23 14:50:53carltongibsonsetmessages: + msg355225
2019-10-23 14:46:31vstinnersetmessages: + msg355224
2019-10-23 14:43:13carltongibsonsetmessages: + msg355222
2019-10-23 14:28:41vstinnersetnosy: + vstinner
messages: + msg355220
2019-10-23 14:27:17vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request16438
2019-10-23 14:18:55vstinnersettitle: Asyncio regression on Windows with Python 3.8 -> ProactorEventLoop.close(): set_wakeup_fd() only works in main thread
2019-10-23 13:12:37asvetlovsetversions: + Python 3.9
2019-10-23 13:12:30asvetlovsetmessages: + msg355217
2019-10-23 12:41:38felixxmsetnosy: + felixxm
2019-10-23 12:22:43carltongibsonsetnosy: - felixxm
messages: + msg355216
2019-10-23 12:19:39felixxmsetnosy: + felixxm
2019-10-23 12:13:28carltongibsoncreate