GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-135953: Properly obtain main thread identifier in Gecko Collector by flowln · Pull Request #146045 · python/cpython

Conversation

flowln commented Mar 17, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Since running a profiler via CLI (python -m profiling.sampling run) spawns a new subprocess where the actual user-specified code will run, a call to threading.main_thread() in the collector's process will not return the profiled process's main thread.

To combat this, we add a new attribute to the returned InterpreterState for the sampled stack frames, which takes the id of the thread_main member from the original sampled interpreter's state.

This allows the main thread to be highlighted on Firefox Profiler:

Issue: Expose internal stack introspection APIs as a statistical runtime analysis tool #135953

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          profiling(Gecko): Properly obtain main thread identifier

4a82ac9

Since running a profiler via CLI (python -m profiling.sampling run)
spawns a new subprocess where the actual user-specified code will run,
a call to threading.main_thread() in the collector's process will not
return the profiled process's main thread.

To combat this, we rely on the fact that thread objects are inserted
in such a way that the first object in the list represents the oldest
ThreadState object [1], which corresponds to a ThreadState associated
with the main thread.

[1] - https://github.com/python/cpython/blob/1b118353bb0a9d816de6ef673f3b11775de5bec5/Include/internal/pycore_interp_structs.h#L831

Signed-off-by: Sofia Donato Ferreira <[email protected]>

flowln requested a review from pablogsal as a code owner

March 17, 2026 02:12

bedevere-app Bot commented Mar 17, 2026

Copy link

Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

python-cla-bot Bot commented Mar 17, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

All commit authors signed the Contributor License Agreement.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot added the awaiting review label

Mar 17, 2026

bedevere-app Bot mentioned this pull request

Mar 17, 2026

Expose internal stack introspection APIs as a statistical runtime analysis tool #135953

Open


          profiling(gecko): take last thread as main thread instead of first one

8920f2f

The ordering is actually newest -> oldest, since the _remote_debugging
code traverses the ThreadState linked list in the intepreter state, appending
to a list of threads in-order.

Signed-off-by: Sofia Donato Ferreira <[email protected]>

bedevere-app Bot commented Mar 17, 2026

Copy link

Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          profiling(Gecko): check if thread list is not empty before accessing it

d9b0a79

It seems to sometimes happen that some samples (possibly at the very start / end)
do not have any active threads in the interpreter state.

It is OK to make main_tid maybe None here, since if it is, the threads list is
empty, and so the for loop will not execute anyways.

Signed-off-by: Sofia Donato Ferreira <[email protected]>

bedevere-app Bot commented Mar 17, 2026

Copy link

Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pablogsal commented Mar 20, 2026

Copy link

Copy Markdown

Member

Thanks for the PR!

I think we should do this but I find the approach a bit flaky. We have enough information in the C layer to know what thread is the main thread so we can surface that in the data that we emit (or alternatively we can add to the unwinder a new method that tells us the tid of the main thread).

Do you feel comfortable to do this in the C code?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          profiling(gecko): retrieve main thread id directly from interpreter s…

f186d3f

…tate

Signed-off-by: Sofia Donato Ferreira <[email protected]>

bedevere-app Bot commented Mar 21, 2026

Copy link

Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

flowln commented Mar 21, 2026

Copy link

Copy Markdown

Contributor Author

@pablogsal I've made the minimum set of changes in the _remote_debugging module to get the main thread id information to the Python side. Since I don't have much of an idea how this module relates to other functionality within the project, please tell me if you see anything that needs changing!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          profiling(gecko): add main_thread_id to MockInterpreter and add check…

5849aad

… in Gecko test

Signed-off-by: Sofia Donato Ferreira <[email protected]>

bedevere-app Bot commented Mar 21, 2026

Copy link

Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pablogsal requested changes

Mar 22, 2026

View reviewed changes

pablogsal left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. Moving the main-thread identification into C is the right direction, but I don't think this version is ready to merge yet.

There are two correctness issues with the current shape:

main_thread_id can remain unset in the live unwinder path. In module.c it is only populated if the loop actually visits the interpreter's main PyThreadState, but in filtered modes (CPU, GIL, EXCEPTION) unwind_stack_for_thread() can skip that thread entirely, and the only_active_thread / targeted-thread paths can avoid it as well.
The new InterpreterInfo.main_thread_id field is not wired through binary replay. InterpreterInfo now has 3 fields, but the binary writer does not serialize main_thread_id, and the binary reader only fills items 0 and 2. That means replay ... --format gecko will still lose the main-thread marker.

I think the cleanest fix is to model this per thread rather than per interpreter: either add an is_main_thread field on ThreadInfo or, better, a new THREAD_STATUS_* bit. That avoids depending on the main thread being present in a given sample, and it should flow through the existing binary status byte naturally.

If you prefer to keep the current main_thread_id approach, then I think we at least need to: use Py_None when the value is unknown, extend the binary format reader/writer for the new field, and add tests for binary-to-Gecko replay plus a filtered-mode case.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot removed the awaiting review label

Mar 22, 2026

bedevere-app Bot commented Mar 22, 2026

Copy link

Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot added the awaiting changes label

Mar 22, 2026

pablogsal commented Mar 22, 2026

Copy link

Copy Markdown

Member

I can give this a go, don't worry

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          profiling(gecko): use status flag for main thread instead of Interpre…

ad5b78c

…terInfo field

Replace the main_thread_id field in InterpreterInfo with a
THREAD_STATUS_MAIN_THREAD status flag set directly in the thread
unwinding code. This simplifies the struct from 3 to 2 fields and
removes the need to compare thread IDs at the Python level.

bedevere-app Bot commented Mar 22, 2026

Copy link

Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pablogsal approved these changes

Mar 22, 2026

View reviewed changes

pablogsal left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I made the marking via a new status bit, which nicely goes through the binary format :)

Thanks a lot for the PR

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

flowln reacted with heart emoji

All reactions

❤️ 1 reaction

bedevere-app Bot added awaiting merge and removed awaiting changes labels

Mar 22, 2026


          Add news entry

acbbd15

pablogsal enabled auto-merge (squash)

March 22, 2026 18:24

pablogsal merged commit a17301a into python:main

Mar 22, 2026

50 checks passed

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the awaiting merge label

Mar 22, 2026

CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request

Mar 23, 2026


          Merge branch 'main' of github.com:python/cpython into fix-pythongh-13…

3e05fa3

…8577

* 'main' of github.com:python/cpython:
  pythongh-146197: Run -m test.pythoninfo on the Emscripten CI (python#146332)
  pythongh-146325: Use `test.support.requires_fork` in test_fastpath_cache_cleared_in_forked_child (python#146330)
  pythongh-146197: Add Emscripten to CI (python#146198)
  pythongh-143387: Raise an exception instead of returning None when metadata file is missing. (python#146234)
  pythongh-108907: ctypes: Document _type_ codes (pythonGH-145837)
  pythongh-146175: Soft-deprecate outdated macros; convert internal usage (pythonGH-146178)
  pythongh-146056: Rework ref counting in treebuilder_handle_end() (python#146167)
  Add a warning about untrusted input to `configparser` docs (python#146276)
  pythongh-145264: Do not ignore excess Base64 data after the first padded quad (pythonGH-145267)
  pythongh-146308: Fix error handling issues in _remote_debugging module (python#146309)
  pythongh-146192: Add base32 support to binascii (pythonGH-146193)
  pythongh-135953: Properly obtain main thread identifier in Gecko Collector (python#146045)
  pythongh-143414: Implement unique reference tracking for JIT, optimize unpacking of such tuples (pythonGH-144300)
  pythongh-146261: Fix bug in `_Py_uop_sym_set_func_version` (pythonGH-146291)
  pythongh-145144: Add more tests for UserList, UserDict, etc (pythonGH-145145)
  pythongh-143959: Fix test_datetime if _datetime is unavailable (pythonGH-145248)
  pythongh-146245: Fix reference and buffer leaks via audit hook in socket module (pythonGH-146248)
  pythongh-140049: Colorize exception notes in `traceback.py` (python#140051)
  Update docs for pythongh-146056 (pythonGH-146213)

pablogsal mentioned this pull request

Apr 4, 2026

gh-130273: Fix traceback color output with unicode characters #142529

Merged

ljfp pushed a commit to ljfp/cpython that referenced this pull request

Apr 25, 2026


          pythongh-135953: Properly obtain main thread identifier in Gecko Coll…

92090c7

…ector (python#146045)

Reviewers

pablogsal

pablogsal approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

flowln commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app Bot commented Mar 17, 2026

Uh oh!

python-cla-bot Bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app Bot commented Mar 17, 2026

Uh oh!

bedevere-app Bot commented Mar 17, 2026

Uh oh!

pablogsal commented Mar 20, 2026

Uh oh!

bedevere-app Bot commented Mar 21, 2026

Uh oh!

flowln commented Mar 21, 2026

Uh oh!

bedevere-app Bot commented Mar 21, 2026

Uh oh!

pablogsal left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Mar 22, 2026

Uh oh!

pablogsal commented Mar 22, 2026

Uh oh!

bedevere-app Bot commented Mar 22, 2026

Uh oh!

pablogsal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

flowln commented Mar 17, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

python-cla-bot Bot commented Mar 17, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.