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

fix(core): add level of indirection for provider.py contextvars #10525

Open
wants to merge 42 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
5109269
Make contextvars indirect
sanchda Sep 5, 2024
918714d
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 5, 2024
219c118
Widen scope for the wrapper
sanchda Sep 5, 2024
d1e2ba2
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 5, 2024
ee448ab
Improve wrapper type
sanchda Sep 5, 2024
f4efab9
Fix wrapper
sanchda Sep 5, 2024
8fc3a4f
Init contextvar on first use rather than eagerly
sanchda Sep 5, 2024
54b9183
Simplify ContextVar wrapper
sanchda Sep 5, 2024
c987426
Drop comment
sanchda Sep 5, 2024
8f2aedb
Merge branch 'main' into sanchda/make_contextvars_indirect
taegyunkim Sep 6, 2024
a159f5c
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 6, 2024
4f47862
Fix typo
sanchda Sep 6, 2024
34292d1
Merge branch 'sanchda/make_contextvars_indirect' of github.com:DataDo…
sanchda Sep 6, 2024
92ee1c5
simplify
sanchda Sep 6, 2024
df4e9aa
Add note
sanchda Sep 6, 2024
f88254d
Improve comment
sanchda Sep 6, 2024
e668f39
spelling
sanchda Sep 6, 2024
e4ea059
Add the wrapper back in
sanchda Sep 6, 2024
bef7417
Take out intermediate variables
sanchda Sep 6, 2024
00b0007
Fix stupid default
sanchda Sep 6, 2024
b45d5ec
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 6, 2024
da70bd4
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 7, 2024
d3d3fcf
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 7, 2024
4756012
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 9, 2024
1801679
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 19, 2024
746d6ce
Merge branch 'main' into sanchda/make_contextvars_indirect
taegyunkim Sep 19, 2024
e2b25fa
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 25, 2024
cd6586a
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 27, 2024
708d8f4
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 30, 2024
f1b411d
Update releasenotes/notes/fix-contextvar-cloning-49adaf7fdf36e8fb.yaml
sanchda Sep 30, 2024
c070f43
Update ddtrace/_trace/provider.py
sanchda Sep 30, 2024
b91dbf2
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Sep 30, 2024
8428719
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 1, 2024
b15dbe8
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 1, 2024
b0ef0ba
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 3, 2024
86c1f2d
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 3, 2024
fb99299
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 3, 2024
09f2302
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 3, 2024
7c57995
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 3, 2024
b661288
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 3, 2024
36c4835
Merge branch 'main' into sanchda/make_contextvars_indirect
sanchda Oct 4, 2024
0712bbc
Merge branch 'main' into sanchda/make_contextvars_indirect
taegyunkim Oct 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 40 additions & 3 deletions ddtrace/_trace/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,46 @@
log = get_logger(__name__)


_DD_CONTEXTVAR: contextvars.ContextVar[Optional[Union[Context, Span]]] = contextvars.ContextVar(
"datadog_contextvar", default=None
)
class ContextVarManager:
sanchda marked this conversation as resolved.
Show resolved Hide resolved
"""
In the implementation of ContextVar, when a key is re-associated with a new value the underlying HAMT must clone
a level of the tree in order to maintain immutability. This operation requires de-referencing pointers to Python
objects stored in the Context, which typically includes objects not created or managed by this library. It's
possible for such objects to have mis-managed reference counts (speculatively: in order to convert their
ContextVar storage from a strong to a weak reference). When such objects are de-referenced--as they would be when
a reassoc from this code forces a clone--it could cause heap corruption or a segmentation fault.

Accordingly, we try to prevent reassoc events when possible by storing a long-lived wrapper object and only setting
the target value within that object.

tl;dr: Don't call `set()` on a context var a second time.
"""

def __init__(self) -> None:
pass

def get(self) -> Optional[Union[Context, Span]]:
wrapper = _DD_ACTUAL_CONTEXTVAR.get()
if wrapper is None:
return None
return wrapper.value

def set(self, value: Optional[Union[Context, Span]]) -> None:
wrapper = _DD_ACTUAL_CONTEXTVAR.get()
if wrapper is None:
wrapper = ContextVarWrapper()
_DD_ACTUAL_CONTEXTVAR.set(wrapper)
wrapper.value = value


class ContextVarWrapper:
def __init__(self) -> None:
self.value: Optional[Union[Context, Span]] = None


# Initialize the context manager
_DD_ACTUAL_CONTEXTVAR = contextvars.ContextVar[Optional[ContextVarWrapper]]("datadog_contextvar", default=None)
_DD_CONTEXTVAR = ContextVarManager()


class BaseContextProvider(metaclass=abc.ABCMeta):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
fixes:
- |
Fixes an issue where normal library use could cause items stored by other libraries to be dereferenced by the
runtime, which could cause crashes if those elements had been garbage collected.
Loading