Repository navigation
gh-148891: prevent crash in HAMT - #148928
Open
KowalskiThomas wants to merge 5 commits into
Open
KowalskiThomas wants to merge 5 commits into
KowalskiThomas wants to merge 5 commits into
Conversation
Eclips4
reviewed
Apr 24, 2026
Comment on lines
+794
to
+803
| // gh-148891: _PyHamt_Assoc can allocate HAMT nodes, which may trigger | ||
| // GC. A weakref callback or finalizer could then re-enter | ||
| // contextvar_set, replacing ctx->ctx_vars via Py_SETREF and freeing | ||
| // the old HAMT while _PyHamt_Assoc is still traversing it. | ||
| // Prevent this by holding a strong reference to the current vars. | ||
| PyHamtObject *old_vars = ctx->ctx_vars; | ||
| Py_INCREF(old_vars); | ||
| PyHamtObject *new_vars = _PyHamt_Assoc( | ||
| ctx->ctx_vars, (PyObject *)var, val); | ||
| old_vars, (PyObject *)var, val); | ||
| Py_DECREF(old_vars); |
Member
There was a problem hiding this comment.
This looks true. We have a lot of increfs/decrefs like that because the call between them can do scary things.
Contributor
Author
|
Updated/Rebased the branch because an unrelated CI test was failing. |
KowalskiThomas
force-pushed
the
kowalski/fix-prevent-crash-in-hamt
branch
from
April 29, 2026 09:15
39aa50f to
d419a0d
Compare
KowalskiThomas
marked this pull request as ready for review
May 13, 2026 10:24
This was referenced Jun 23, 2026
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
to DataDog/dd-trace-py
that referenced
this pull request
Jun 24, 2026
## Description This is an attempt to fix a customer-reported crash ([Jira](https://datadoghq.atlassian.net/browse/SCP-1205)). - The customer is using a version that already includes this `ContextVar`-related crash: #17407 - It's likely python/cpython#148928 would fix the crash as well but (1) it would only land in recent versions of CPython -- the customer is using 3.11 -- and (2) I don't have a review/approval yet. This fix addresses the issue in a sort of makeshift way like the CPython upstream fix would. Since we can't bump reference counts directly from CPython, the PR adds a Rust helper which takes care of it. Not great, not terrible. **Note** the bug only happens in versions prior to 3.12 (which made significant changes to the GC and doesn't trigger that HAMT crash anymore), so the fix is gated to them. --- The crash signature is ``` Error UnixSignal: Process terminated with SEGV_MAPERR (SIGSEGV) #0 0x00000000006a3e3b #1 0x0000000000690b5b #2 0x00000000004a5e01 #3 0x0000000000690a27 #4 0x0000000000690990 #5 0x0000000000690699 PyContextVar_Set #6 0x0000000000540be3 _PyEval_EvalFrameDefault #7 0x0000000000585937 #8 0x000000000058512e #9 0x000000000054aea5 PyObject_Vectorcall #10 0x000000000053d7a6 _PyEval_EvalFrameDefault #11 0x0000000000585040 #12 0x0000000000541c19 _PyEval_EvalFrameDefault #13 0x0000000000583177 #14 0x0000000000581a14 #15 0x00000000005b418d #16 0x000000000053fb96 _PyEval_EvalFrameDefault #17 0x00000000005afd4e #18 0x00000000005bf334 #19 0x000000000053f6b2 _PyEval_EvalFrameDefault #20 0x0000000000585937 #21 0x000000000058512e #22 0x0000000000570534 PyObject_Call #23 0x0000000000541c19 _PyEval_EvalFrameDefault #24 0x0000000000565f43 _PyFunction_Vectorcall #25 0x00000000005341e9 _PyObject_FastCallDictTstate #26 0x000000000056e0f9 _PyObject_Call_Prepend #27 0x000000000065a02b #28 0x000000000052f59c _PyObject_MakeTpCall #29 0x000000000053d7a6 _PyEval_EvalFrameDefault #30 0x00000000006140f4 #31 0x0000000000613757 PyEval_EvalCode #32 0x00000000006344eb #33 0x0000000000630744 #34 0x0000000000644be5 #35 0x00000000006441f4 _PyRun_SimpleFileObject #36 0x0000000000644007 _PyRun_AnyFileObject #37 0x000000000063ed19 Py_RunMain #38 0x00000000006049cd Py_BytesMain #39 0x00007f3bd11a6d90 __libc_start_call_main (sysdeps/nptl/libc_start_call_main.h:58) #40 0x0000000000000000 call_init (csu/libc-start.c:128) #41 0x00007f3bd11a6e40 call_init (csu/libc-start.c:128) #42 0x00007f3bd11a6e40 __libc_start_main_alias_2 (csu/libc-start.c:379) #43 0x0000000000604855 _start ``` Co-authored-by: thomas.kowalski <thomas.kowalski@datadoghq.com>
|
This PR is stale because it has been open for 90 days with no activity. |
KowalskiThomas
force-pushed
the
kowalski/fix-prevent-crash-in-hamt
branch
from
October 21, 2026 08:52
d419a0d to
1971be8
Compare
cmaloney
approved these changes
Oct 21, 2026
KowalskiThomas
commented
Oct 21, 2026
Contributor
|
@1st1 could do a review of this? |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What is this PR?
Fixes #148891.
_PyHamt_Assocallocates HAMT nodes, which can trigger GC. A weakref callback or finaliser fired during GC could re-entercontextvar_set, replacingctx->ctx_varsviaPy_SETREFand freeing the old HAMT while_PyHamt_Assocwas still traversing it, causing a use-after-free.The proposed fix is to hold a strong reference to
ctx->ctx_varsduring the_PyHamt_Assoccall, so that a re-entrant free cannot affect the HAMT being traversed.