Skip to content

loop.call_soon_threadsafe should be documented to be re-entrant-safe too #79149

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

Closed
njsmith opened this issue Oct 13, 2018 · 6 comments
Closed
Labels
3.10 only security fixes 3.11 only security fixes 3.12 only security fixes 3.13 bugs and security fixes docs Documentation in the Doc dir topic-asyncio

Comments

@njsmith
Copy link
Contributor

njsmith commented Oct 13, 2018

BPO 34968
Nosy @njsmith, @asvetlov, @1st1

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 = None
created_at = <Date 2018-10-13.02:20:02.537>
labels = ['3.8', 'expert-asyncio']
title = 'loop.call_soon_threadsafe should be documented to be re-entrant-safe too'
updated_at = <Date 2018-10-14.07:19:46.306>
user = 'https://github.com/njsmith'

bugs.python.org fields:

activity = <Date 2018-10-14.07:19:46.306>
actor = 'njs'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2018-10-13.02:20:02.537>
creator = 'njs'
dependencies = []
files = []
hgrepos = []
issue_num = 34968
keywords = []
message_count = 4.0
messages = ['327618', '327640', '327652', '327693']
nosy_count = 3.0
nosy_names = ['njs', 'asvetlov', 'yselivanov']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue34968'
versions = ['Python 3.8']

Linked PRs

@njsmith
Copy link
Contributor Author

njsmith commented Oct 13, 2018

Asyncio needs a way to schedule work from other threads; and it also needs a way to scheduler work from code that can run at arbitrary times in the same thread, such as signal handlers or object finalizers ("reentrant contexts").

Currently loop.call_soon_threadsafe is documented to be the way to schedule work from other threads, but there is no documented way to schedule work from reentrant contexts. These are not quite the same thing, because reentrant contexts block the main thread while they're running. Generally, making code safe to call from __del__ or signal handlers is strictly harder than making it safe to call from other threads. (See bpo-14976 for an example of stdlib code being thread-safe but not reentrant-safe.)

Technically speaking, this means that right now, if you need to call an asyncio API from a __del__ method, then the only officially supported way to do that is to write something like:

def __del__(self):
    def actual_cleanup_code():
        ...
    def thread_dispatcher():
        loop.call_soon_threadsafe(actual_cleanup_code)
    thread = threading.Thread(target=thread_dispatcher)
    thread.start()

But this is kind of silly. There should be some equivalent of loop.call_soon that *is* safe to call from reentrant contexts, so we could just write:

def __del__(self):
    def actual_cleanup_code():
        ...
    loop.call_soon_reentrant_safe(actual_cleanup_code)

But... it doesn't really make sense to add a new method for this, since the desired semantics are strictly more powerful than the current loop.call_soon_threadsafe. Instead, we should tighten the guarantees on call_soon_threadsafe, by documenting that it's safe to use from reentrant contexts.

Also, AFAICT the stdlib's implementation of call_soon_threadsafe is already reentrant-safe, so this wouldn't require any changes to stdlib code, only to the docs. But it would provide an additional formal guarantee that user-level code could take advantage of, and impose an additional constraint on developers of third-party loops.

(I don't think the constraint is *too* onerous, fortunately. It's quite tricky to implement a version of call_soon that's thread-safe, reentrant-safe, *and* guarantees that the callback will eventually be invoked, even if call_soon races with loop shutdown. But in asyncio, all variants of call_soon are allowed to silently drop callbacks at loop shutdown, which makes this much easier.)

@asvetlov
Copy link
Contributor

Agree.
Docs update sounds good.
Nathaniel, would you create a docs patch?

@1st1
Copy link
Member

1st1 commented Oct 13, 2018

AFAICT the stdlib's implementation of call_soon_threadsafe is already reentrant-safe

What would make it not reentrant-safe? We'll need to document that for the benefit of asyncio and third-party maintainers.

@njsmith
Copy link
Contributor Author

njsmith commented Oct 14, 2018

What would make it not reentrant-safe?

Probably the most obvious example of a non-reentrant-safe operation is taking a lock. It's very natural to write code like:

def call_soon(...):
    with self._call_soon_lock:
        ...

but now imagine that inside the body of that 'with' statement, a signal arrives, so the interpreter pauses what it's doing to invoke the signal handler, and the signal handler turns around and invokes call_soon, which tries to acquire the same lock that it already holds → instant deadlock.

And this rules out quite a few of the tools you might normally expect to use in thread-safe code, like queue.Queue, since they use locks internally.

The reason I think the stdlib's call_soon is OK is that it doesn't perform any blocking operations, and the critical operation is simply 'self._ready.append(...)', which in CPython is atomic with respect to threads/signals/GC.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@graingert
Copy link
Contributor

asyncio.run now requires that call_soon_threadsafe is safe to call from a signal handler:

def _on_sigint(self, signum, frame, main_task):
self._interrupt_count += 1
if self._interrupt_count == 1 and not main_task.done():
main_task.cancel()
# wakeup loop if it is blocked by select() with long timeout
self._loop.call_soon_threadsafe(lambda: None)
return
raise KeyboardInterrupt()

graingert added a commit to graingert/cpython that referenced this issue Jul 4, 2022
@ezio-melotti ezio-melotti moved this to Todo in asyncio Jul 17, 2022
@kumaraditya303 kumaraditya303 added docs Documentation in the Doc dir 3.11 only security fixes 3.10 only security fixes 3.12 only security fixes and removed 3.8 (EOL) end of life labels Oct 9, 2022
@willingc
Copy link
Contributor

@graingert I'm going through and triaging docs issues. I'm going to unassign you from this. If you would like to tackle, please reassign; otherwise, let's make this free game for the community.

@willingc willingc added the 3.13 bugs and security fixes label Oct 19, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 9, 2025
…fe` (pythonGH-128662)

(cherry picked from commit 4685401)

Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Thomas Grainger <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 9, 2025
…fe` (pythonGH-128662)

(cherry picked from commit 4685401)

Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Thomas Grainger <[email protected]>
@github-project-automation github-project-automation bot moved this from Todo to Done in asyncio Jan 9, 2025
kumaraditya303 added a commit that referenced this issue Jan 9, 2025
…afe` (GH-128662) (#128664)

gh-79149: document reentrant safety of `loop.call_soon_threadsafe` (GH-128662)
(cherry picked from commit 4685401)

Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Thomas Grainger <[email protected]>
kumaraditya303 added a commit that referenced this issue Jan 9, 2025
…afe` (GH-128662) (#128665)

gh-79149: document reentrant safety of `loop.call_soon_threadsafe` (GH-128662)
(cherry picked from commit 4685401)

Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Thomas Grainger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 only security fixes 3.11 only security fixes 3.12 only security fixes 3.13 bugs and security fixes docs Documentation in the Doc dir topic-asyncio
Projects
Status: Done
Development

No branches or pull requests

6 participants