Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| 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: |
| 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) | ||
Uh oh!
There was an error while loading. Please reload this page.
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.