Skip to content

Commit

Permalink
fix GIL holder inference for memory attributions
Browse files Browse the repository at this point in the history
  • Loading branch information
P403n1x87 committed Aug 20, 2023
1 parent 2aca8c9 commit 570f3d6
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/argparse.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
#else
#define DEFAULT_SAMPLING_INTERVAL 100
#endif
#define DEFAULT_INIT_TIMEOUT_MS 500 // 0.5 seconds
#define DEFAULT_INIT_TIMEOUT_MS 1000 // 1 second
#define DEFAULT_HEAP_SIZE 0

const char SAMPLE_FORMAT_NORMAL[] = ";%s:%s:%d";
Expand Down
16 changes: 14 additions & 2 deletions src/py_proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1215,13 +1215,25 @@ py_proc__sample(py_proc_t * self) {

if (pargs.memory) {
// Use the current thread to determine which thread is manipulating memory
current_thread = _py_proc__get_current_thread_state_raddr(self);
if (V_MIN(3, 12)) {
void * gil_state_raddr = V_FIELD(void *, is, py_is, o_gil_state);

Check warning on line 1219 in src/py_proc.c

View check run for this annotation

Codecov / codecov/patch

src/py_proc.c#L1219

Added line #L1219 was not covered by tests
if (!isvalid(gil_state_raddr))
SUCCESS;

Check warning on line 1221 in src/py_proc.c

View check run for this annotation

Codecov / codecov/patch

src/py_proc.c#L1221

Added line #L1221 was not covered by tests
gil_state_t gil_state;
if (fail(copy_datatype(self->proc_ref, gil_state_raddr, gil_state))) {
log_ie("Failed to copy GIL state");
FAIL;

Check warning on line 1225 in src/py_proc.c

View check run for this annotation

Codecov / codecov/patch

src/py_proc.c#L1224-L1225

Added lines #L1224 - L1225 were not covered by tests
}
current_thread = (void *) gil_state.last_holder._value;

Check warning on line 1227 in src/py_proc.c

View check run for this annotation

Codecov / codecov/patch

src/py_proc.c#L1227

Added line #L1227 was not covered by tests
}
else
current_thread = _py_proc__get_current_thread_state_raddr(self);
}

do {
if (pargs.memory) {
mem_delta = 0;
if (self->symbols[DYNSYM_RUNTIME] != NULL && current_thread == (void *) -1) {
if (V_MAX(3, 11) && self->symbols[DYNSYM_RUNTIME] != NULL && current_thread == (void *) -1) {
if (_py_proc__find_current_thread_offset(self, py_thread.raddr.addr))
continue;
else
Expand Down
4 changes: 3 additions & 1 deletion src/py_thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,9 @@ py_thread__fill_from_raddr(py_thread_t * self, raddr_t * raddr, py_proc_t * proc
self->raddr = *raddr;

self->top_frame = V_FIELD(void*, ts, py_thread, o_frame);


self->status = V_FIELD(tstate_status_t, ts, py_thread, o_status);

self->next_raddr = (raddr_t) {
raddr->pref,
V_FIELD(void*, ts, py_thread, o_next) == raddr->addr \
Expand Down
2 changes: 2 additions & 0 deletions src/py_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ typedef struct thread {
/* The per-thread datastack was introduced in Python 3.11 */
void * stack;
size_t stack_size;

tstate_status_t status;
} py_thread_t;


Expand Down
11 changes: 2 additions & 9 deletions src/python/gil.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,8 @@ struct _gil_runtime_state3_11 {
unsigned long interval;
_Py_atomic_address last_holder;
_Py_atomic_int locked;
unsigned long switch_number;
PyCOND_T cond;
PyMUTEX_T mutex;
#ifdef FORCE_SWITCHING
/* This condition variable helps the GIL-releasing thread wait for
a GIL-awaiting thread to be scheduled and take the GIL. */
PyCOND_T switch_cond;
PyMUTEX_T switch_mutex;
#endif
};

typedef struct _gil_runtime_state3_11 gil_state_t;

#endif
10 changes: 9 additions & 1 deletion src/python/interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ typedef struct {
size_t stacksize;
} threads;

struct pyruntimestate *runtime;
void *runtime;

_Py_atomic_address _finalizing;

Expand All @@ -160,6 +160,14 @@ typedef struct {

// Dictionary of the builtins module
PyObject *builtins;

struct {
_Py_atomic_int eval_breaker;
_Py_atomic_int gil_drop_request;
int recursion_limit;
void *gil;
// ...
} ceval;
} PyInterpreterState3_12;


Expand Down
50 changes: 26 additions & 24 deletions src/python/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,37 +148,39 @@ typedef struct _ts3_11 {
} PyThreadState3_11;


typedef struct {
/* Has been initialized to a safe state.
In order to be effective, this must be set to 0 during or right
after allocation. */
unsigned int initialized:1;

/* Has been bound to an OS thread. */
unsigned int bound:1;
/* Has been unbound from its OS thread. */
unsigned int unbound:1;
/* Has been bound aa current for the GILState API. */
unsigned int bound_gilstate:1;
/* Currently in use (maybe holds the GIL). */
unsigned int active:1;

/* various stages of finalization */
unsigned int finalizing:1;
unsigned int cleared:1;
unsigned int finalized:1;

/* padding to align to 4 bytes */
unsigned int :24;
} tstate_status_t;

typedef struct {
/* See Python/ceval.c for comments explaining most fields */

void *prev;
void *next;
void *interp;

struct {
/* Has been initialized to a safe state.
In order to be effective, this must be set to 0 during or right
after allocation. */
unsigned int initialized:1;

/* Has been bound to an OS thread. */
unsigned int bound:1;
/* Has been unbound from its OS thread. */
unsigned int unbound:1;
/* Has been bound aa current for the GILState API. */
unsigned int bound_gilstate:1;
/* Currently in use (maybe holds the GIL). */
unsigned int active:1;

/* various stages of finalization */
unsigned int finalizing:1;
unsigned int cleared:1;
unsigned int finalized:1;

/* padding to align to 4 bytes */
unsigned int :24;
} _status;
tstate_status_t _status;

int py_recursion_remaining;
int py_recursion_limit;
Expand Down
25 changes: 23 additions & 2 deletions src/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ typedef struct {
offset_t o_thread_id;
offset_t o_native_thread_id;
offset_t o_stack;
offset_t o_status;
} py_thread_v;


Expand Down Expand Up @@ -162,6 +163,7 @@ typedef struct {
offset_t o_next;
offset_t o_tstate_head;
offset_t o_gc;
offset_t o_gil_state;
} py_is_v;


Expand Down Expand Up @@ -264,6 +266,18 @@ typedef struct {
offsetof(s, datastack_chunk), \
}

#define PY_THREAD_312(s) { \
sizeof(s), \
offsetof(s, prev), \
offsetof(s, next), \
offsetof(s, interp), \
offsetof(s, cframe), \
offsetof(s, thread_id), \
offsetof(s, native_thread_id), \
offsetof(s, datastack_chunk), \
offsetof(s, _status) \
}

#define PY_UNICODE(n) { \
n \
}
Expand Down Expand Up @@ -298,6 +312,13 @@ typedef struct {
offsetof(s, gc), \
}

#define PY_IS_312(s) { \
sizeof(s), \
offsetof(s, next), \
offsetof(s, threads.head), \
offsetof(s, gc), \
offsetof(s, ceval.gil), \
}

#define PY_GC(s) { \
sizeof(s), \
Expand Down Expand Up @@ -356,8 +377,8 @@ python_v python_v3_11 = {
python_v python_v3_12 = {
PY_CODE_311 (PyCodeObject3_12),
PY_FRAME (PyFrameObject3_10), // Irrelevant
PY_THREAD_311 (PyThreadState3_12),
PY_IS_311 (PyInterpreterState3_12),
PY_THREAD_312 (PyThreadState3_12),
PY_IS_312 (PyInterpreterState3_12),
PY_RUNTIME_311 (_PyRuntimeState3_12),
PY_GC (struct _gc_runtime_state3_12),
PY_CFRAME_311 (_PyCFrame3_12),
Expand Down
2 changes: 1 addition & 1 deletion test/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_cli_no_python():
"-C",
"powershell" if platform.system() == "Windows" else "bash",
"-c",
"sleep 1",
"sleep 2",
)
assert result.returncode == 39
assert "not a Python" in result.stderr or "Cannot launch" in result.stderr
Expand Down

0 comments on commit 570f3d6

Please sign in to comment.