Skip to content

Commit a17301a

Browse files
authored
gh-135953: Properly obtain main thread identifier in Gecko Collector (#146045)
1 parent e36f8db commit a17301a

File tree

8 files changed

+65
-14
lines changed

8 files changed

+65
-14
lines changed

Lib/profiling/sampling/constants.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
THREAD_STATUS_UNKNOWN,
3838
THREAD_STATUS_GIL_REQUESTED,
3939
THREAD_STATUS_HAS_EXCEPTION,
40+
THREAD_STATUS_MAIN_THREAD,
4041
)
4142
except ImportError:
4243
# Fallback for tests or when module is not available
@@ -45,3 +46,4 @@
4546
THREAD_STATUS_UNKNOWN = (1 << 2)
4647
THREAD_STATUS_GIL_REQUESTED = (1 << 3)
4748
THREAD_STATUS_HAS_EXCEPTION = (1 << 4)
49+
THREAD_STATUS_MAIN_THREAD = (1 << 5)

Lib/profiling/sampling/gecko_collector.py

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@
99
from .collector import Collector, filter_internal_frames
1010
from .opcode_utils import get_opcode_info, format_opcode
1111
try:
12-
from _remote_debugging import THREAD_STATUS_HAS_GIL, THREAD_STATUS_ON_CPU, THREAD_STATUS_UNKNOWN, THREAD_STATUS_GIL_REQUESTED, THREAD_STATUS_HAS_EXCEPTION
12+
from _remote_debugging import THREAD_STATUS_HAS_GIL, THREAD_STATUS_ON_CPU, THREAD_STATUS_UNKNOWN, THREAD_STATUS_GIL_REQUESTED, THREAD_STATUS_HAS_EXCEPTION, THREAD_STATUS_MAIN_THREAD
1313
except ImportError:
1414
# Fallback if module not available (shouldn't happen in normal use)
1515
THREAD_STATUS_HAS_GIL = (1 << 0)
1616
THREAD_STATUS_ON_CPU = (1 << 1)
1717
THREAD_STATUS_UNKNOWN = (1 << 2)
1818
THREAD_STATUS_GIL_REQUESTED = (1 << 3)
1919
THREAD_STATUS_HAS_EXCEPTION = (1 << 4)
20+
THREAD_STATUS_MAIN_THREAD = (1 << 5)
2021

2122

2223
# Categories matching Firefox Profiler expectations
@@ -174,15 +175,16 @@ def collect(self, stack_frames, timestamps_us=None):
174175
for thread_info in interpreter_info.threads:
175176
frames = filter_internal_frames(thread_info.frame_info)
176177
tid = thread_info.thread_id
178+
status_flags = thread_info.status
179+
is_main_thread = bool(status_flags & THREAD_STATUS_MAIN_THREAD)
177180

178181
# Initialize thread if needed
179182
if tid not in self.threads:
180-
self.threads[tid] = self._create_thread(tid)
183+
self.threads[tid] = self._create_thread(tid, is_main_thread)
181184

182185
thread_data = self.threads[tid]
183186

184187
# Decode status flags
185-
status_flags = thread_info.status
186188
has_gil = bool(status_flags & THREAD_STATUS_HAS_GIL)
187189
on_cpu = bool(status_flags & THREAD_STATUS_ON_CPU)
188190
gil_requested = bool(status_flags & THREAD_STATUS_GIL_REQUESTED)
@@ -288,18 +290,12 @@ def collect(self, stack_frames, timestamps_us=None):
288290

289291
self.sample_count += len(times)
290292

291-
def _create_thread(self, tid):
293+
def _create_thread(self, tid, is_main_thread):
292294
"""Create a new thread structure with processed profile format."""
293295

294-
# Determine if this is the main thread
295-
try:
296-
is_main = tid == threading.main_thread().ident
297-
except (RuntimeError, AttributeError):
298-
is_main = False
299-
300296
thread = {
301297
"name": f"Thread-{tid}",
302-
"isMainThread": is_main,
298+
"isMainThread": is_main_thread,
303299
"processStartupTime": 0,
304300
"processShutdownTime": None,
305301
"registerTime": 0,

Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818
THREAD_STATUS_UNKNOWN,
1919
THREAD_STATUS_GIL_REQUESTED,
2020
THREAD_STATUS_HAS_EXCEPTION,
21+
THREAD_STATUS_MAIN_THREAD,
2122
)
2223
from profiling.sampling.binary_collector import BinaryCollector
2324
from profiling.sampling.binary_reader import BinaryReader
25+
from profiling.sampling.gecko_collector import GeckoCollector
2426

2527
ZSTD_AVAILABLE = _remote_debugging.zstd_available()
2628
except ImportError:
@@ -318,6 +320,7 @@ def test_status_flags_preserved(self):
318320
THREAD_STATUS_UNKNOWN,
319321
THREAD_STATUS_GIL_REQUESTED,
320322
THREAD_STATUS_HAS_EXCEPTION,
323+
THREAD_STATUS_MAIN_THREAD,
321324
THREAD_STATUS_HAS_GIL | THREAD_STATUS_ON_CPU,
322325
THREAD_STATUS_HAS_GIL | THREAD_STATUS_HAS_EXCEPTION,
323326
THREAD_STATUS_HAS_GIL
@@ -342,6 +345,35 @@ def test_status_flags_preserved(self):
342345
self.assertEqual(count, len(statuses))
343346
self.assert_samples_equal(samples, collector)
344347

348+
def test_binary_replay_preserves_main_thread_for_gecko(self):
349+
"""Binary replay preserves main thread identity for GeckoCollector."""
350+
samples = [
351+
[
352+
make_interpreter(
353+
0,
354+
[
355+
make_thread(
356+
1,
357+
[make_frame("main.py", 10, "main")],
358+
THREAD_STATUS_MAIN_THREAD,
359+
),
360+
make_thread(2, [make_frame("worker.py", 20, "worker")]),
361+
],
362+
)
363+
]
364+
]
365+
filename = self.create_binary_file(samples)
366+
collector = GeckoCollector(1000)
367+
368+
with BinaryReader(filename) as reader:
369+
count = reader.replay_samples(collector)
370+
371+
self.assertEqual(count, 2)
372+
profile = collector._build_profile()
373+
threads = {thread["tid"]: thread for thread in profile["threads"]}
374+
self.assertTrue(threads[1]["isMainThread"])
375+
self.assertFalse(threads[2]["isMainThread"])
376+
345377
def test_multiple_threads_per_sample(self):
346378
"""Multiple threads in one sample roundtrip exactly."""
347379
threads = [

Lib/test/test_profiling/test_sampling_profiler/test_collectors.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
THREAD_STATUS_HAS_GIL,
2929
THREAD_STATUS_ON_CPU,
3030
THREAD_STATUS_GIL_REQUESTED,
31+
THREAD_STATUS_MAIN_THREAD,
3132
)
3233
except ImportError:
3334
raise unittest.SkipTest(
@@ -524,6 +525,7 @@ def test_gecko_collector_basic(self):
524525
MockThreadInfo(
525526
1,
526527
[MockFrameInfo("file.py", 10, "func1"), MockFrameInfo("file.py", 20, "func2")],
528+
status=THREAD_STATUS_MAIN_THREAD,
527529
)
528530
],
529531
)
@@ -556,6 +558,7 @@ def test_gecko_collector_basic(self):
556558
threads = profile_data["threads"]
557559
self.assertEqual(len(threads), 1)
558560
thread_data = threads[0]
561+
self.assertTrue(thread_data["isMainThread"])
559562

560563
# Verify thread structure
561564
self.assertIn("samples", thread_data)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Properly identify the main thread in the Gecko profiler collector by
2+
using a status flag from the interpreter state instead of relying on
3+
:func:`threading.main_thread` in the collector process.

Modules/_remote_debugging/_remote_debugging.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ typedef enum _WIN32_THREADSTATE {
172172
#define THREAD_STATUS_UNKNOWN (1 << 2)
173173
#define THREAD_STATUS_GIL_REQUESTED (1 << 3)
174174
#define THREAD_STATUS_HAS_EXCEPTION (1 << 4)
175+
#define THREAD_STATUS_MAIN_THREAD (1 << 5)
175176

176177
/* Exception cause macro */
177178
#define set_exception_cause(unwinder, exc_type, message) \
@@ -575,7 +576,8 @@ extern PyObject* unwind_stack_for_thread(
575576
RemoteUnwinderObject *unwinder,
576577
uintptr_t *current_tstate,
577578
uintptr_t gil_holder_tstate,
578-
uintptr_t gc_frame
579+
uintptr_t gc_frame,
580+
uintptr_t main_thread_tstate
579581
);
580582

581583
/* Thread stopping functions (for blocking mode) */

Modules/_remote_debugging/module.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,11 +583,16 @@ _remote_debugging_RemoteUnwinder_get_stack_trace_impl(RemoteUnwinderObject *self
583583
current_tstate = self->tstate_addr;
584584
}
585585

586+
// Acquire main thread state information
587+
uintptr_t main_thread_tstate = GET_MEMBER(uintptr_t, interp_state_buffer,
588+
self->debug_offsets.interpreter_state.threads_main);
589+
586590
while (current_tstate != 0) {
587591
uintptr_t prev_tstate = current_tstate;
588592
PyObject* frame_info = unwind_stack_for_thread(self, &current_tstate,
589593
gil_holder_tstate,
590-
gc_frame);
594+
gc_frame,
595+
main_thread_tstate);
591596
if (!frame_info) {
592597
// Check if this was an intentional skip due to mode-based filtering
593598
if ((self->mode == PROFILING_MODE_CPU || self->mode == PROFILING_MODE_GIL ||
@@ -1207,6 +1212,9 @@ _remote_debugging_exec(PyObject *m)
12071212
if (PyModule_AddIntConstant(m, "THREAD_STATUS_HAS_EXCEPTION", THREAD_STATUS_HAS_EXCEPTION) < 0) {
12081213
return -1;
12091214
}
1215+
if (PyModule_AddIntConstant(m, "THREAD_STATUS_MAIN_THREAD", THREAD_STATUS_MAIN_THREAD) < 0) {
1216+
return -1;
1217+
}
12101218

12111219
if (RemoteDebugging_InitState(st) < 0) {
12121220
return -1;

Modules/_remote_debugging/threads.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,8 @@ unwind_stack_for_thread(
291291
RemoteUnwinderObject *unwinder,
292292
uintptr_t *current_tstate,
293293
uintptr_t gil_holder_tstate,
294-
uintptr_t gc_frame
294+
uintptr_t gc_frame,
295+
uintptr_t main_thread_tstate
295296
) {
296297
PyObject *frame_info = NULL;
297298
PyObject *thread_id = NULL;
@@ -395,6 +396,10 @@ unwind_stack_for_thread(
395396
status_flags |= THREAD_STATUS_ON_CPU;
396397
}
397398

399+
if (*current_tstate == main_thread_tstate) {
400+
status_flags |= THREAD_STATUS_MAIN_THREAD;
401+
}
402+
398403
// Check if we should skip this thread based on mode
399404
int should_skip = 0;
400405
if (unwinder->skip_non_matching_threads) {

0 commit comments

Comments
 (0)