Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

C implemented Future doesn't work on Windows #72634

Closed
methane opened this issue Oct 15, 2016 · 8 comments
Closed

C implemented Future doesn't work on Windows #72634

methane opened this issue Oct 15, 2016 · 8 comments
Assignees
@methane
Labels
3.7 (EOL) end of life topic-asyncio type-feature A feature request or enhancement

Comments

@methane
Copy link
Member

methane commented Oct 15, 2016

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
    Copy link
    Member Author

    methane commented Oct 15, 2016

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

    @methane methane added the 3.7 (EOL) end of life label Oct 15, 2016
    @methane methane self-assigned this Oct 15, 2016
    @methane methane added topic-asyncio type-feature A feature request or enhancement labels Oct 15, 2016
    @1st1
    Copy link
    Member

    1st1 commented Oct 17, 2016

    C implemented Future should allow overriding _schedule_callbacks.

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

    @methane
    Copy link
    Member Author

    methane commented Oct 18, 2016

    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
    Copy link
    Member Author

    methane commented Oct 18, 2016

    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
    Copy link
    Member

    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
    Copy link
    Member Author

    methane commented Oct 19, 2016

    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
    Copy link
    Member

    1st1 commented Oct 20, 2016

    Latest patch looks good.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 21, 2016

    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

    @methane methane closed this as completed Oct 21, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @methane methane

    Labels
    3.7 (EOL) end of life topic-asyncio type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @methane @1st1