classification
Title: C implemented Future doesn't work on Windows
Type: enhancement Stage: resolved
Components: asyncio Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: methane Nosy List: gvanrossum, methane, python-dev, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2016-10-15 02:09 by methane, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.

Files
File name Uploaded Description Edit
schedule_callbacks.patch methane, 2016-10-17 18:19 review
dont-override-schedule-callbacks.patch methane, 2016-10-19 00:52 review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft, 2017-03-31 16:36
Messages (8)
msg278685 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-10-15 02:09
_WaitCancelFuture in windows_events.py overrides _schedule_callbacks.
C implemented Future should allow overriding _schedule_callbacks.

Since `{"_future", PyInit__future},` is not in PC/config.c, _future is not registered as builtin module. So Python 3.6b2 doesn't use it.
Instead of registering it, we should try make it split extension module (.pyd file).
msg278827 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-10-17 21:34
> C implemented Future should allow overriding _schedule_callbacks.

I'm not so sure about this.  Maybe we can just fix _WaitCancelFuture somehow?
msg278843 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-10-18 05:49
https://github.com/search?p=3&q=_schedule_callbacks&type=Code&utf8=%E2%9C%93

At least, Future class in uvloop have same API.
Most of other results seems just copy of Python source tree.
(But I didn't check all search result)
msg278848 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-10-18 09:11
> I'm not so sure about this.  Maybe we can just fix _WaitCancelFuture somehow?

@haypo, do you know why _WaitCancelFuture overrides _schedule_callbacks() instead
of self.add_done_callback(self._unregister_wait_cb)?
_unregister_wait_cb must be called after all callbacks are called?
msg278881 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-10-18 13:59
INADA Naoki added the comment:
> @haypo, do you know why _WaitCancelFuture overrides _schedule_callbacks() instead
> of self.add_done_callback(self._unregister_wait_cb)?
> _unregister_wait_cb must be called after all callbacks are called?

Oh no. I tried to forget this mess :-( It took me 2 or 3 months to
understand and fix this complex issue of cancelling a wait on
Windows...

Hum, let me check.

I found this in IocpProactor:
---
    def _wait_cancel(self, event, done_callback):
        fut = self._wait_for_handle(event, None, True)
        # add_done_callback() cannot be used because the wait may only complete
        # in IocpProactor.close(), while the event loop is not running.
        fut._done_callback = done_callback
        return fut
---

I don't understand my comment anymore /o\

I just recall that it was complex to get this crap working in all
cases, especially in corner cases.
msg278956 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-10-19 00:52
I think _WaitCancelFuture can do same thing by overriding
callers of _schedule_callbacks.

Attached patch does it, and make _schedule_callbacks private
by renaming it to __schedule_callbacks.
msg279078 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-10-20 20:24
Latest patch looks good.
msg279106 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-21 03:33
New changeset 6d20d6fe9b41 by INADA Naoki in branch '3.6':
Issue #28448: Fix C implemented asyncio.Future didn't work on Windows
https://hg.python.org/cpython/rev/6d20d6fe9b41

New changeset a9a136c9d857 by INADA Naoki in branch 'default':
Issue #28448: Fix C implemented asyncio.Future didn't work on Windows (merge 3.6)
https://hg.python.org/cpython/rev/a9a136c9d857
History
Date User Action Args
2017-03-31 16:36:31dstufftsetpull_requests: + pull_request1047
2016-10-21 03:33:49methanesetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-10-21 03:33:12python-devsetnosy: + python-dev
messages: + msg279106
2016-10-20 20:24:55yselivanovsetmessages: + msg279078
2016-10-19 00:52:55methanesetfiles: + dont-override-schedule-callbacks.patch

messages: + msg278956
2016-10-18 13:59:19vstinnersetmessages: + msg278881
2016-10-18 09:11:59methanesetnosy: + vstinner
messages: + msg278848
2016-10-18 05:49:01methanesetmessages: + msg278843
2016-10-17 21:34:37yselivanovsetmessages: + msg278827
2016-10-17 18:19:31methanesetfiles: + schedule_callbacks.patch
keywords: + patch
stage: needs patch -> patch review
2016-10-15 02:09:28methanecreate