-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
gh-145254: Add thread safety annotation #145255
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
Merged
kumaraditya303
merged 4 commits into
python:main
from
lysnikolaou:thread-safety-annotation
Mar 12, 2026
+195
−0
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # Thread safety annotations for C API functions. | ||
| # | ||
| # Each line has the form: | ||
| # function_name : level | ||
| # | ||
| # Where level is one of: | ||
| # incompatible -- not safe even with external locking | ||
| # compatible -- safe if the caller serializes all access with external locks | ||
| # distinct -- safe on distinct objects without external synchronization | ||
| # shared -- safe for concurrent use on the same object | ||
| # atomic -- atomic | ||
| # | ||
| # Lines beginning with '#' are ignored. | ||
| # The function name must match the C domain identifier used in the documentation. | ||
|
|
||
| # Synchronization primitives (Doc/c-api/synchronization.rst) | ||
| PyMutex_Lock:shared: | ||
| PyMutex_Unlock:shared: | ||
| PyMutex_IsLocked:atomic: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,12 @@ | |
| * Reference count annotations for C API functions. | ||
| * Stable ABI annotations | ||
| * Limited API annotations | ||
| * Thread safety annotations for C API functions. | ||
|
|
||
| Configuration: | ||
| * Set ``refcount_file`` to the path to the reference count data file. | ||
| * Set ``stable_abi_file`` to the path to stable ABI list. | ||
| * Set ``threadsafety_file`` to the path to the thread safety data file. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
@@ -48,6 +50,15 @@ class RefCountEntry: | |
| result_refs: int | None = None | ||
|
|
||
|
|
||
| @dataclasses.dataclass(frozen=True, slots=True) | ||
| class ThreadSafetyEntry: | ||
| # Name of the function. | ||
| name: str | ||
| # Thread safety level. | ||
| # One of: 'incompatible', 'compatible', 'safe'. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe use literal instead of str |
||
| level: str | ||
|
|
||
|
|
||
| @dataclasses.dataclass(frozen=True, slots=True) | ||
| class StableABIEntry: | ||
| # Role of the object. | ||
|
|
@@ -113,10 +124,42 @@ def read_stable_abi_data(stable_abi_file: Path) -> dict[str, StableABIEntry]: | |
| return stable_abi_data | ||
|
|
||
|
|
||
| _VALID_THREADSAFETY_LEVELS = frozenset({ | ||
| "incompatible", | ||
| "compatible", | ||
| "distinct", | ||
| "shared", | ||
| "atomic", | ||
| }) | ||
|
|
||
|
|
||
| def read_threadsafety_data( | ||
| threadsafety_filename: Path, | ||
| ) -> dict[str, ThreadSafetyEntry]: | ||
| threadsafety_data = {} | ||
| for line in threadsafety_filename.read_text(encoding="utf8").splitlines(): | ||
| line = line.strip() | ||
| if not line or line.startswith("#"): | ||
| continue | ||
| # Each line is of the form: function_name : level : [comment] | ||
| parts = line.split(":", 2) | ||
| if len(parts) < 2: | ||
| raise ValueError(f"Wrong field count in {line!r}") | ||
| name, level = parts[0].strip(), parts[1].strip() | ||
| if level not in _VALID_THREADSAFETY_LEVELS: | ||
| raise ValueError( | ||
| f"Unknown thread safety level {level!r} for {name!r}. " | ||
| f"Valid levels: {sorted(_VALID_THREADSAFETY_LEVELS)}" | ||
| ) | ||
| threadsafety_data[name] = ThreadSafetyEntry(name=name, level=level) | ||
| return threadsafety_data | ||
|
|
||
|
|
||
| def add_annotations(app: Sphinx, doctree: nodes.document) -> None: | ||
| state = app.env.domaindata["c_annotations"] | ||
| refcount_data = state["refcount_data"] | ||
| stable_abi_data = state["stable_abi_data"] | ||
| threadsafety_data = state["threadsafety_data"] | ||
| for node in doctree.findall(addnodes.desc_content): | ||
| par = node.parent | ||
| if par["domain"] != "c": | ||
|
|
@@ -126,6 +169,12 @@ def add_annotations(app: Sphinx, doctree: nodes.document) -> None: | |
| name = par[0]["ids"][0].removeprefix("c.") | ||
| objtype = par["objtype"] | ||
|
|
||
| # Thread safety annotation — inserted first so it appears last (bottom-most) | ||
| # among all annotations. | ||
| if entry := threadsafety_data.get(name): | ||
| annotation = _threadsafety_annotation(entry.level) | ||
| node.insert(0, annotation) | ||
|
|
||
| # Stable ABI annotation. | ||
| if record := stable_abi_data.get(name): | ||
| if ROLE_TO_OBJECT_TYPE[record.role] != objtype: | ||
|
|
@@ -256,6 +305,46 @@ def _unstable_api_annotation() -> nodes.admonition: | |
| ) | ||
|
|
||
|
|
||
| def _threadsafety_annotation(level: str) -> nodes.emphasis: | ||
| match level: | ||
| case "incompatible": | ||
| display = sphinx_gettext("Not safe to call from multiple threads.") | ||
| reftarget = "threadsafety-level-incompatible" | ||
| case "compatible": | ||
| display = sphinx_gettext( | ||
| "Safe to call from multiple threads" | ||
| " with external synchronization only." | ||
| ) | ||
| reftarget = "threadsafety-level-compatible" | ||
| case "distinct": | ||
| display = sphinx_gettext( | ||
| "Safe to call without external synchronization" | ||
| " on distinct objects." | ||
| ) | ||
| reftarget = "threadsafety-level-distinct" | ||
| case "shared": | ||
| display = sphinx_gettext( | ||
| "Safe for concurrent use on the same object." | ||
| ) | ||
| reftarget = "threadsafety-level-shared" | ||
| case "atomic": | ||
| display = sphinx_gettext("Atomic.") | ||
| reftarget = "threadsafety-level-atomic" | ||
| case _: | ||
| raise AssertionError(f"Unknown thread safety level {level!r}") | ||
| ref_node = addnodes.pending_xref( | ||
| display, | ||
| nodes.Text(display), | ||
| refdomain="std", | ||
| reftarget=reftarget, | ||
| reftype="ref", | ||
| refexplicit="True", | ||
| ) | ||
| prefix = sphinx_gettext("Thread safety:") + " " | ||
| classes = ["threadsafety", f"threadsafety-{level}"] | ||
| return nodes.emphasis("", prefix, ref_node, classes=classes) | ||
|
|
||
|
|
||
| def _return_value_annotation(result_refs: int | None) -> nodes.emphasis: | ||
| classes = ["refcount"] | ||
| if result_refs is None: | ||
|
|
@@ -342,11 +431,15 @@ def init_annotations(app: Sphinx) -> None: | |
| state["stable_abi_data"] = read_stable_abi_data( | ||
| Path(app.srcdir, app.config.stable_abi_file) | ||
| ) | ||
| state["threadsafety_data"] = read_threadsafety_data( | ||
| Path(app.srcdir, app.config.threadsafety_file) | ||
| ) | ||
|
|
||
|
|
||
| def setup(app: Sphinx) -> ExtensionMetadata: | ||
| app.add_config_value("refcount_file", "", "env", types={str}) | ||
| app.add_config_value("stable_abi_file", "", "env", types={str}) | ||
| app.add_config_value("threadsafety_file", "", "env", types={str}) | ||
| app.add_directive("limited-api-list", LimitedAPIList) | ||
| app.add_directive("corresponding-type-slot", CorrespondingTypeSlot) | ||
| app.connect("builder-inited", init_annotations) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another good example for this is APIs that return borrowed references
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all of them :)
It would be nice to use specific functions as examples, like with
PyList_GetItemRefandPyMutex_IsLockedbelow.