C implemented Future doesn't work on Windows

[methane]

BPO	28448
Nosy	@gvanrossum, @vstinner, @methane, @1st1
PRs	[Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
Files	schedule_callbacks.patch dont-override-schedule-callbacks.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/methane'
closed_at = <Date 2016-10-21.03:33:49.021>
created_at = <Date 2016-10-15.02:09:27.802>
labels = ['type-feature', '3.7', 'expert-asyncio']
title = "C implemented Future doesn't work on Windows"
updated_at = <Date 2017-03-31.16:36:31.803>
user = 'https://github.com/methane'

bugs.python.org fields:

activity = <Date 2017-03-31.16:36:31.803>
actor = 'dstufft'
assignee = 'methane'
closed = True
closed_date = <Date 2016-10-21.03:33:49.021>
closer = 'methane'
components = ['asyncio']
creation = <Date 2016-10-15.02:09:27.802>
creator = 'methane'
dependencies = []
files = ['45125', '45139']
hgrepos = []
issue_num = 28448
keywords = ['patch']
message_count = 8.0
messages = ['278685', '278827', '278843', '278848', '278881', '278956', '279078', '279106']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'vstinner', 'methane', 'python-dev', 'yselivanov']
pr_nums = ['552']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue28448'
versions = ['Python 3.6', 'Python 3.7']

[methane]

_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).

[1st1]

C implemented Future should allow overriding _schedule_callbacks.

I'm not so sure about this. Maybe we can just fix _WaitCancelFuture somehow?

[methane]

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)

[methane]

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?

[vstinner]

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.

[methane]

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.

[1st1]

Latest patch looks good.

[python-dev]

New changeset 6d20d6fe9b41 by INADA Naoki in branch '3.6':
Issue bpo-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 bpo-28448: Fix C implemented asyncio.Future didn't work on Windows (merge 3.6)
https://hg.python.org/cpython/rev/a9a136c9d857