Navigation Menu

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

Protect tasks weak set manipulation in asyncio.all_tasks() #79151

Closed
asvetlov opened this issue Oct 13, 2018 · 13 comments
Closed

Protect tasks weak set manipulation in asyncio.all_tasks() #79151

asvetlov opened this issue Oct 13, 2018 · 13 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-asyncio

Comments

@asvetlov
Copy link
Contributor

BPO 34970
Nosy @ned-deily, @asvetlov, @1st1, @miss-islington
PRs
  • bpo-34970: Protect tasks weak set manipulation in asyncio.all_tasks() #9837
  • [3.7] bpo-34970: Protect tasks weak set manipulation in asyncio.all_tasks() (GH-9837) #9849
    		•

    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 = None
closed_at = <Date 2018-10-20.17:07:37.062>
created_at = <Date 2018-10-13.08:11:44.726>
labels = ['3.7', '3.8', 'expert-asyncio']
title = 'Protect tasks weak set manipulation in asyncio.all_tasks()'
updated_at = <Date 2018-10-20.17:07:37.061>
user = 'https://github.com/asvetlov'

    bugs.python.org fields:

    activity = <Date 2018-10-20.17:07:37.061>
actor = 'ned.deily'
assignee = 'none'
closed = True
closed_date = <Date 2018-10-20.17:07:37.062>
closer = 'ned.deily'
components = ['asyncio']
creation = <Date 2018-10-13.08:11:44.726>
creator = 'asvetlov'
dependencies = []
files = []
hgrepos = []
issue_num = 34970
keywords = ['patch']
message_count = 13.0
messages = ['327632', '327633', '327635', '327636', '327642', '327651', '327660', '327662', '327663', '327666', '327667', '328167', '328168']
nosy_count = 4.0
nosy_names = ['ned.deily', 'asvetlov', 'yselivanov', 'miss-islington']
pr_nums = ['9837', '9849']
priority = None
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue34970'
versions = ['Python 3.7', 'Python 3.8']
    @asvetlov
    Copy link
    Contributor Author 

    Traceback (most recent call last):
  File "C:\Users\max\AppData\Local\Programs\Python\Python37\lib\threading.py", line 917, in _bootstrap_inner
    self.run()
  File "D:\Proj\antwork\code\antwork_backend\server\data_handlers\order\playback_hd.py", line 147, in run
    send_to_ws(channel=self.room_name, data=json.loads(msg))
  File "D:\Proj\antwork\code\antwork_backend\server\data_exchange.py", line 26, in send_to_ws
    asyncio.run(channel_layer.group_send(channel, send_data))
  File "C:\Users\max\AppData\Local\Programs\Python\Python37\lib\asyncio\runners.py", line 46, in run
    _cancel_all_tasks(loop)
  File "C:\Users\max\AppData\Local\Programs\Python\Python37\lib\asyncio\runners.py", line 54, in _cancel_all_tasks
    to_cancel = tasks.all_tasks(loop)
  File "C:\Users\max\AppData\Local\Programs\Python\Python37\lib\asyncio\tasks.py", line 38, in all_tasks
    return {t for t in _all_tasks
  File "C:\Users\max\AppData\Local\Programs\Python\Python37\lib\asyncio\tasks.py", line 38, in <setcomp>
    return {t for t in _all_tasks
  File "C:\Users\max\AppData\Local\Programs\Python\Python37\lib\_weakrefset.py", line 61, in __iter__
    for itemref in self.data:
RuntimeError: Set changed size during iteration

    416c1eb#commitcomment-30887909

    @asvetlov asvetlov changed the title Protect all_tasks manipulation in asyncio.all_tasks() Protect tasks weak set manipulation in asyncio.all_tasks() Oct 13, 2018
    @asvetlov
    Copy link
    Contributor Author

    The issue is hard to reproduce, the problem happens on an implicit garbage collector run during iteration over _all_tasks weakset.

    @asvetlov
    Copy link
    Contributor Author

    Ned, would be nice to have this trivial bugfix in 3.7.1.

    The problem happens if wakes up on weakset iteration.
    The chance is very low, it leads to a flaky scary bug.

    @ned-deily
    Copy link
    Member

    3.7.1rc2 is about to be released so to add this would require retagging and remanufacturing release bits. If the chances of it happening are "very low", I would prefer to not further delay rc2. If you and Yury agree that the risk is low and the benefit is high, we could consider risking cherry-picking it to 3.7.1 final. Sound OK?

    @asvetlov
    Copy link
    Contributor Author

    Let's wait for Yuri opinion.

    @1st1
    Copy link
    Member

    1st1 commented Oct 13, 2018

    > we could consider risking cherry-picking it to 3.7.1 final. Sound OK?
    Let's wait for Yuri opinion.

    I agree it's a pretty serious bug; basically a time bomb that can crash a perfectly fine asyncio application. The PR itself seems to be safe to cherry-pick to 3.7.1 even after rc2. I suggest to merge it asap though to have some time for buildbots to test it.

    @miss-islington
    Copy link
    Contributor

    New changeset 97cf082 by Miss Islington (bot) (Andrew Svetlov) in branch 'master':
    bpo-34970: Protect tasks weak set manipulation in asyncio.all_tasks() (GH-9837)
    97cf082

    @asvetlov
    Copy link
    Contributor Author

    New changeset 5dbb1b7 by Andrew Svetlov (Miss Islington (bot)) in branch '3.7':
    bpo-34970: Protect tasks weak set manipulation in asyncio.all_tasks() (GH-9837) (GH-9849)
    5dbb1b7

    @asvetlov
    Copy link
    Contributor Author

    Ned, the fix has landed on both master and 3.7

    Your turn, please.

    @ned-deily
    Copy link
    Member

    Thanks, Andrew and Yury! Just leave the issue open as a "release blocker" for now and I will take care of it for 3.7.1 final.

    @asvetlov
    Copy link
    Contributor Author

    Cool!
    Thanks, Ned

    @ned-deily
    Copy link
    Member

    New changeset 60c663c by Ned Deily (Miss Islington (bot)) in branch '3.7':
    bpo-34970: Protect tasks weak set manipulation in asyncio.all_tasks() (GH-9837) (GH-9849)
    60c663c

    @ned-deily
    Copy link
    Member

    Released in 3.7.1

    @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
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes topic-asyncio
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @1st1 @asvetlov @ned-deily @miss-islington