Skip to content

Commit 8593c48

Browse files
yoneymeta-codesync[bot]
authored andcommitted
Fix data race on CodeExtra::calls for FT Python
Summary: Fix TSAN-reported data race on extra->calls by introducing atomic accessor functions for FT-Python. ThreadSanitizer: data race fbcode/cinderx/Interpreter/3.14/Includes/generated_cases.c.h:4249 in Ci_EvalFrame ================== ``` - After No race warnings. Reviewed By: DinoV Differential Revision: D93088007 fbshipit-source-id: 086c562d3bc73fd0286eaabfd17d87443666d4f1
1 parent 2735fb4 commit 8593c48

6 files changed

Lines changed: 43 additions & 7 deletions

File tree

cinderx/Common/code_extra.h

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
#include <stdint.h>
66

7+
#ifdef __cplusplus
8+
extern "C" {
9+
#endif
10+
711
// Extra data attached to a code object.
812
typedef struct CodeExtra {
913
union {
@@ -13,3 +17,35 @@ typedef struct CodeExtra {
1317
struct CodeExtra* next;
1418
};
1519
} CodeExtra;
20+
21+
// Thread-safe accessors for CodeExtra::calls.
22+
// Under FT-Python, these use atomics to avoid data races.
23+
#ifdef Py_GIL_DISABLED
24+
25+
// Note: _Py_atomic_add_uint64 uses seq_cst ordering, which might be stronger
26+
// than needed for the calls counter. On x86-64, this is the same cost as
27+
// relaxed (both emit lock xaddq). On ARM, a relaxed variant would be cheaper
28+
// but there is no _Py_atomic_add_uint64_relaxed.
29+
static inline void Ci_code_extra_incr_calls(CodeExtra* extra) {
30+
_Py_atomic_add_uint64(&extra->calls, 1);
31+
}
32+
33+
static inline uint64_t Ci_code_extra_get_calls(const CodeExtra* extra) {
34+
return _Py_atomic_load_uint64_relaxed(&extra->calls);
35+
}
36+
37+
#else
38+
39+
static inline void Ci_code_extra_incr_calls(CodeExtra* extra) {
40+
extra->calls += 1;
41+
}
42+
43+
static inline uint64_t Ci_code_extra_get_calls(const CodeExtra* extra) {
44+
return extra->calls;
45+
}
46+
47+
#endif
48+
49+
#ifdef __cplusplus
50+
}
51+
#endif

cinderx/Interpreter/3.14/Includes/ceval_macros.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ do { \
453453
if (extra == NULL) { \
454454
adaptive_enabled = false; \
455455
} else { \
456-
extra->calls += 1; \
456+
Ci_code_extra_incr_calls(extra); \
457457
adaptive_enabled = is_adaptive_enabled(extra); \
458458
} \
459459
} \

cinderx/Interpreter/3.14/interpreter.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ bool Ci_DelayAdaptiveCode = false;
3838
uint64_t Ci_AdaptiveThreshold = 80;
3939

4040
bool is_adaptive_enabled(CodeExtra *extra) {
41-
return !Ci_DelayAdaptiveCode || extra->calls > Ci_AdaptiveThreshold;
41+
return !Ci_DelayAdaptiveCode || Ci_code_extra_get_calls(extra) > Ci_AdaptiveThreshold;
4242
}
4343

4444
#endif

cinderx/Interpreter/3.15/interpreter.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ bool Ci_DelayAdaptiveCode = false;
3535
uint64_t Ci_AdaptiveThreshold = 80;
3636

3737
bool is_adaptive_enabled(CodeExtra *extra) {
38-
return !Ci_DelayAdaptiveCode || extra->calls > Ci_AdaptiveThreshold;
38+
return !Ci_DelayAdaptiveCode || Ci_code_extra_get_calls(extra) > Ci_AdaptiveThreshold;
3939
}
4040

4141
#endif
@@ -454,7 +454,7 @@ Py_ssize_t load_method_static_cached_oparg_slot(int oparg) {
454454
if (extra == NULL) { \
455455
adaptive_enabled = false; \
456456
} else { \
457-
extra->calls += 1; \
457+
Ci_code_extra_incr_calls(extra); \
458458
adaptive_enabled = is_adaptive_enabled(extra); \
459459
} \
460460
} \
@@ -547,7 +547,7 @@ void **opcode_targets = opcode_targets_table;
547547

548548
// Suppress unused variable warning because it's too hard to improve the
549549
// variable's scope to avoid an unused-but-set-variable warning.
550-
(void)adaptive_enabled;
550+
(void)adaptive_enabled;
551551

552552
/* support for generator.throw() */
553553
if (throwflag) {

cinderx/Jit/pyjit.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ uint64_t countCalls(PyCodeObject* code) {
102102
return code->co_mutable->ncalls;
103103
#else
104104
auto extra = codeExtra(code);
105-
return extra != nullptr ? extra->calls : 0;
105+
return extra != nullptr ? Ci_code_extra_get_calls(extra) : 0;
106106
#endif
107107
}
108108

cinderx/RuntimeTests/backend_test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ for i in range(30):
297297
#else
298298
auto extra = codeExtra(code);
299299
ASSERT_NE(extra, nullptr) << "Failed to load code object extra data";
300-
uint64_t ncalls = extra->calls;
300+
uint64_t ncalls = Ci_code_extra_get_calls(extra);
301301
#endif
302302

303303
// TASK(T190615535): This is waiting on the 3.12 custom interpreter loop.

0 commit comments

Comments
 (0)