Skip to content

Commit a2ae327

Browse files
author
Saul Cooperman
committed
Address feedback
1 parent 77a8d89 commit a2ae327

8 files changed

Lines changed: 184 additions & 138 deletions

File tree

‎src/pystack/__main__.py‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,16 +285,23 @@ def process_remote(parser: argparse.ArgumentParser, args: argparse.Namespace) ->
285285
if not args.block and args.native_mode != NativeReportingMode.OFF:
286286
parser.error("Native traces are only available in blocking mode")
287287

288-
printer = TracebackPrinter(
289-
native_mode=args.native_mode, include_subinterpreters=True
290-
)
291-
for thread in get_process_threads(
288+
threads = get_process_threads(
292289
args.pid,
293290
stop_process=args.block,
294291
native_mode=args.native_mode,
295292
locals=args.locals,
296293
method=StackMethod.ALL if args.exhaustive else StackMethod.AUTO,
297-
):
294+
)
295+
296+
has_multiple_subinterpreters = (
297+
len(set(thread.interpreter_id for thread in threads)) > 1
298+
)
299+
300+
printer = TracebackPrinter(
301+
native_mode=args.native_mode,
302+
include_subinterpreters=has_multiple_subinterpreters,
303+
)
304+
for thread in threads:
298305
printer.print_thread(thread)
299306

300307

@@ -417,15 +424,24 @@ def process_core(parser: argparse.ArgumentParser, args: argparse.Namespace) -> N
417424
elf_id if elf_id else "<MISSING>",
418425
)
419426

420-
printer = TracebackPrinter(args.native_mode, include_subinterpreters=True)
421-
for thread in get_process_threads_for_core(
427+
threads = get_process_threads_for_core(
422428
corefile,
423429
executable,
424430
library_search_path=lib_search_path,
425431
native_mode=args.native_mode,
426432
locals=args.locals,
427433
method=StackMethod.ALL if args.exhaustive else StackMethod.AUTO,
428-
):
434+
)
435+
436+
has_multiple_subinterpreters = (
437+
len(set(thread.interpreter_id for thread in threads)) > 1
438+
)
439+
440+
printer = TracebackPrinter(
441+
args.native_mode, include_subinterpreters=has_multiple_subinterpreters
442+
)
443+
444+
for thread in threads:
429445
printer.print_thread(thread)
430446

431447

‎src/pystack/_pystack.pyx‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -505,20 +505,7 @@ def _entry_frame_count(thread: PyThread) -> int:
505505
return sum(1 for frame in thread.all_frames if frame.is_entry)
506506

507507

508-
def _eval_frame_positions(thread: PyThread):
509-
if not thread.python_version:
510-
return []
511-
return [
512-
index
513-
for index, native_frame in enumerate(thread.native_frames)
514-
if frame_type(native_frame, thread.python_version) == NativeFrame.FrameType.EVAL
515-
]
516-
517-
518508
def _slice_native_stacks_for_same_tid_threads(threads) -> None:
519-
if len(threads) < 2:
520-
return
521-
522509
canonical = next((thread for thread in threads if thread.native_frames), None)
523510
if canonical is None:
524511
return

‎src/pystack/_pystack/pythread.cpp‎

Lines changed: 63 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -55,88 +55,79 @@ findPthreadTidOffset(
5555
remote_addr_t interp_state_addr)
5656
{
5757
LOG(DEBUG) << "Attempting to locate tid offset in pthread structure";
58+
Structure<py_is_v> is(manager, interp_state_addr);
5859

59-
// If interp_state_addr does not point to the main interpreter (id 0) we won't find the
60-
// PID == TID in the interpreter threads. Hence, we traverse the linked list of interpreters. The
61-
// main interpreter is not necessarily the head of the linked lists of interpreters.
60+
auto current_thread_addr = is.getField(&py_is_v::o_tstate_head);
6261

63-
while (interp_state_addr != 0) {
64-
Structure<py_is_v> is(manager, interp_state_addr);
62+
auto thread_head = current_thread_addr;
6563

66-
auto current_thread_addr = is.getField(&py_is_v::o_tstate_head);
67-
68-
auto thread_head = current_thread_addr;
69-
70-
// Iterate over all Python threads until we find a thread that has a tid equal to
71-
// the process pid. This works because in the main thread the tid is equal to the pid,
72-
// so when this happens it has to happen on the main thread. Note that the main thread
73-
// is not necessarily at the head of the Python thread linked list
64+
// Iterate over all Python threads until we find a thread that has a tid equal to
65+
// the process pid. This works because in the main thread the tid is equal to the pid,
66+
// so when this happens it has to happen on the main thread. Note that the main thread
67+
// is not necessarily at the head of the Python thread linked list
7468

7569
#if defined(__GLIBC__)
76-
// If we detect GLIBC, we can try the two main known structs for 'struct
77-
// pthread' that we know about to avoid having to do guess-work by doing a
78-
// linear scan over the struct.
79-
while (current_thread_addr != (remote_addr_t) nullptr) {
80-
Structure<py_thread_v> current_thread(manager, current_thread_addr);
81-
auto pthread_id_addr = current_thread.getField(&py_thread_v::o_thread_id);
82-
83-
pid_t the_tid;
84-
std::vector<off_t> glibc_pthread_offset_candidates = {
85-
offsetof(_pthread_structure_with_simple_header, tid),
86-
offsetof(_pthread_structure_with_tcbhead, tid)};
87-
for (off_t candidate : glibc_pthread_offset_candidates) {
88-
manager->copyObjectFromProcess((remote_addr_t)(pthread_id_addr + candidate), &the_tid);
89-
if (the_tid == manager->Pid()) {
90-
LOG(DEBUG) << "Tid offset located using GLIBC offsets at offset " << std::showbase
91-
<< std::hex << candidate << " in pthread structure";
92-
return candidate;
93-
}
94-
}
95-
remote_addr_t next_thread_addr = current_thread.getField(&py_thread_v::o_next);
96-
if (next_thread_addr == current_thread_addr) {
97-
break;
70+
// If we detect GLIBC, we can try the two main known structs for 'struct
71+
// pthread' that we know about to avoid having to do guess-work by doing a
72+
// linear scan over the struct.
73+
while (current_thread_addr != (remote_addr_t) nullptr) {
74+
Structure<py_thread_v> current_thread(manager, current_thread_addr);
75+
auto pthread_id_addr = current_thread.getField(&py_thread_v::o_thread_id);
76+
77+
pid_t the_tid;
78+
std::vector<off_t> glibc_pthread_offset_candidates = {
79+
offsetof(_pthread_structure_with_simple_header, tid),
80+
offsetof(_pthread_structure_with_tcbhead, tid)};
81+
for (off_t candidate : glibc_pthread_offset_candidates) {
82+
manager->copyObjectFromProcess((remote_addr_t)(pthread_id_addr + candidate), &the_tid);
83+
if (the_tid == manager->Pid()) {
84+
LOG(DEBUG) << "Tid offset located using GLIBC offsets at offset " << std::showbase
85+
<< std::hex << candidate << " in pthread structure";
86+
return candidate;
9887
}
99-
current_thread_addr = next_thread_addr;
10088
}
89+
remote_addr_t next_thread_addr = current_thread.getField(&py_thread_v::o_next);
90+
if (next_thread_addr == current_thread_addr) {
91+
break;
92+
}
93+
current_thread_addr = next_thread_addr;
94+
}
10195
#endif
10296

103-
current_thread_addr = thread_head;
104-
105-
while (current_thread_addr != (remote_addr_t) nullptr) {
106-
Structure<py_thread_v> current_thread(manager, current_thread_addr);
107-
auto pthread_id_addr = current_thread.getField(&py_thread_v::o_thread_id);
108-
109-
// Attempt to locate a field in the pthread struct that's equal to the pid.
110-
uintptr_t buffer[100];
111-
size_t buffer_size = sizeof(buffer);
112-
while (buffer_size > 0) {
113-
try {
114-
LOG(DEBUG) << "Trying to copy a buffer of " << buffer_size
115-
<< " bytes to get pthread ID";
116-
manager->copyMemoryFromProcess(pthread_id_addr, buffer_size, &buffer);
117-
break;
118-
} catch (const RemoteMemCopyError& ex) {
119-
LOG(DEBUG) << "Failed to copy buffer to get pthread ID";
120-
buffer_size /= 2;
121-
}
122-
}
123-
LOG(DEBUG) << "Copied a buffer of " << buffer_size << " bytes to get pthread ID";
124-
for (size_t i = 0; i < buffer_size / sizeof(uintptr_t); i++) {
125-
if (static_cast<pid_t>(buffer[i]) == manager->Pid()) {
126-
off_t offset = sizeof(uintptr_t) * i;
127-
LOG(DEBUG) << "Tid offset located by scanning at offset " << std::showbase
128-
<< std::hex << offset << " in pthread structure";
129-
return offset;
130-
}
131-
}
97+
current_thread_addr = thread_head;
98+
99+
while (current_thread_addr != (remote_addr_t) nullptr) {
100+
Structure<py_thread_v> current_thread(manager, current_thread_addr);
101+
auto pthread_id_addr = current_thread.getField(&py_thread_v::o_thread_id);
132102

133-
remote_addr_t next_thread_addr = current_thread.getField(&py_thread_v::o_next);
134-
if (next_thread_addr == current_thread_addr) {
103+
// Attempt to locate a field in the pthread struct that's equal to the pid.
104+
uintptr_t buffer[100];
105+
size_t buffer_size = sizeof(buffer);
106+
while (buffer_size > 0) {
107+
try {
108+
LOG(DEBUG) << "Trying to copy a buffer of " << buffer_size << " bytes to get pthread ID";
109+
manager->copyMemoryFromProcess(pthread_id_addr, buffer_size, &buffer);
135110
break;
111+
} catch (const RemoteMemCopyError& ex) {
112+
LOG(DEBUG) << "Failed to copy buffer to get pthread ID";
113+
buffer_size /= 2;
114+
}
115+
}
116+
LOG(DEBUG) << "Copied a buffer of " << buffer_size << " bytes to get pthread ID";
117+
for (size_t i = 0; i < buffer_size / sizeof(uintptr_t); i++) {
118+
if (static_cast<pid_t>(buffer[i]) == manager->Pid()) {
119+
off_t offset = sizeof(uintptr_t) * i;
120+
LOG(DEBUG) << "Tid offset located by scanning at offset " << std::showbase << std::hex
121+
<< offset << " in pthread structure";
122+
return offset;
136123
}
137-
current_thread_addr = next_thread_addr;
138124
}
139-
interp_state_addr = InterpreterUtils::getNextInterpreter(manager, interp_state_addr);
125+
126+
remote_addr_t next_thread_addr = current_thread.getField(&py_thread_v::o_next);
127+
if (next_thread_addr == current_thread_addr) {
128+
break;
129+
}
130+
current_thread_addr = next_thread_addr;
140131
}
141132
LOG(ERROR) << "Could not find tid offset in pthread structure";
142133
return 0;
@@ -155,8 +146,9 @@ PyThread::PyThread(const std::shared_ptr<const AbstractProcessManager>& manager,
155146
LOG(DEBUG) << std::hex << std::showbase << "Attempting to construct frame from address "
156147
<< frame_addr;
157148
d_first_frame = std::make_unique<FrameObject>(manager, frame_addr, 0);
149+
150+
d_stack_anchor = getStackAnchor(manager, frame_addr);
158151
}
159-
d_stack_anchor = getStackAnchor(manager, frame_addr);
160152

161153
d_addr = addr;
162154
remote_addr_t candidate_next_addr = ts.getField(&py_thread_v::o_next);
@@ -418,7 +410,7 @@ getThreadFromInterpreterState(
418410
const std::shared_ptr<const AbstractProcessManager>& manager,
419411
remote_addr_t addr)
420412
{
421-
if (tid_offset_in_pthread_struct == 0) {
413+
if (tid_offset_in_pthread_struct == 0 && !manager->versionIsAtLeast(3, 11)) {
422414
tid_offset_in_pthread_struct = findPthreadTidOffset(manager, addr);
423415
}
424416

‎src/pystack/_pystack/version.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ py_is()
181181

182182
template<class T>
183183
constexpr py_is_v
184-
py_isv7()
184+
py_isv37()
185185
{
186186
return {
187187
sizeof(T),
@@ -615,7 +615,7 @@ python_v python_v3_7 = {
615615
py_code<Python3_6::PyCodeObject>(),
616616
py_frame<Python3_7::PyFrameObject>(),
617617
py_thread<Python3_7::PyThreadState>(),
618-
py_isv7<Python3_7::PyInterpreterState>(),
618+
py_isv37<Python3_7::PyInterpreterState>(),
619619
py_runtime<Python3_7::PyRuntimeState>(),
620620
py_gc<Python3_7::_gc_runtime_state>(),
621621
};
@@ -637,7 +637,7 @@ python_v python_v3_8 = {
637637
py_code<Python3_8::PyCodeObject>(),
638638
py_frame<Python3_7::PyFrameObject>(),
639639
py_thread<Python3_7::PyThreadState>(),
640-
py_isv7<Python3_8::PyInterpreterState>(),
640+
py_isv37<Python3_8::PyInterpreterState>(),
641641
py_runtime<Python3_8::PyRuntimeState>(),
642642
py_gc<Python3_8::_gc_runtime_state>(),
643643
};
@@ -659,7 +659,7 @@ python_v python_v3_9 = {
659659
py_code<Python3_8::PyCodeObject>(),
660660
py_frame<Python3_7::PyFrameObject>(),
661661
py_thread<Python3_7::PyThreadState>(),
662-
py_isv7<Python3_9::PyInterpreterState>(),
662+
py_isv37<Python3_9::PyInterpreterState>(),
663663
py_runtime<Python3_9::PyRuntimeState>(),
664664
py_gc<Python3_8::_gc_runtime_state>(),
665665
};
@@ -681,7 +681,7 @@ python_v python_v3_10 = {
681681
py_code<Python3_8::PyCodeObject>(),
682682
py_frame<Python3_10::PyFrameObject>(),
683683
py_thread<Python3_7::PyThreadState>(),
684-
py_isv7<Python3_9::PyInterpreterState>(),
684+
py_isv37<Python3_9::PyInterpreterState>(),
685685
py_runtime<Python3_9::PyRuntimeState>(),
686686
py_gc<Python3_8::_gc_runtime_state>(),
687687
};

‎src/pystack/traceback_formatter.py‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,14 @@ def __init__(
1818
):
1919
self.native_mode = native_mode
2020
self.include_subinterpreters = include_subinterpreters
21-
self._current_interpreter_id = -1
21+
self._current_interpreter_id: Optional[int] = None
2222

2323
def print_thread(self, thread: PyThread) -> None:
2424
# Print interpreter header if we've switched interpreters
2525
if self.include_subinterpreters:
2626
if thread.interpreter_id != self._current_interpreter_id:
2727
self._print_interpreter_header(thread.interpreter_id)
28-
self._current_interpreter_id = (
29-
thread.interpreter_id if thread.interpreter_id is not None else -1
30-
)
28+
self._current_interpreter_id = thread.interpreter_id
3129

3230
# Print the thread with indentation
3331
for line in format_thread(thread, self.native_mode):

‎test_file_multiple_interps.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import os
2+
import threading
3+
import time
4+
from concurrent import interpreters
5+
6+
print(os.getpid())
7+
NUM_INTERPRETERS = 3
8+
9+
10+
def start_interpreter_async(interp, code):
11+
t = threading.Thread(target=interp.exec, args=(code,))
12+
t.daemon = True
13+
t.start()
14+
return t
15+
16+
17+
CODE = """
18+
import time
19+
import threading
20+
print(threading.get_native_id())
21+
print(f"Hello from sub-interpreter")
22+
while True:
23+
time.sleep(1)
24+
"""
25+
26+
for i in range(NUM_INTERPRETERS):
27+
interp = interpreters.create()
28+
start_interpreter_async(interp, CODE)
29+
30+
print("Main interpreter sleeping forever...")
31+
while True:
32+
time.sleep(1)

‎tests/integration/test_subinterpreters.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
from collections import Counter
55
from contextlib import redirect_stdout
66
from pathlib import Path
7+
from typing import Dict
8+
from typing import List
79
from typing import Set
810

911
import pytest
@@ -322,7 +324,7 @@ def _assert_native_eval_symbols(threads) -> None:
322324

323325

324326
def _assert_mergeable_same_tid_groups(threads) -> bool:
325-
groups = {}
327+
groups: Dict[int, List] = {}
326328
for thread in threads:
327329
groups.setdefault(thread.tid, []).append(thread)
328330

0 commit comments

Comments
 (0)