Skip to content

Fix greenlet integration with greenlet 3.4.0#908

Merged
godlygeek merged 1 commit into
bloomberg:mainfrom
godlygeek:fix_greenlet_handling
May 10, 2026
Merged

Fix greenlet integration with greenlet 3.4.0#908
godlygeek merged 1 commit into
bloomberg:mainfrom
godlygeek:fix_greenlet_handling

Conversation

@godlygeek
Copy link
Copy Markdown
Contributor

Our attempts to handle ancient versions of greenlet are causing us to try to import the settrace function from greenlet rather than from greenlet._greenlet, which is failing because we're trying to import it before greenlet's own __init__.py has imported it.

Drop support for greenlet versions older than 1.0, which is more than 5 years out of date at this point, to fix this issue.

@godlygeek godlygeek requested a review from a team April 27, 2026 20:58
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.38%. Comparing base (1952221) to head (f739785).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #908      +/-   ##
==========================================
- Coverage   92.49%   92.38%   -0.12%     
==========================================
  Files          99       99              
  Lines       11746    11785      +39     
  Branches      426      429       +3     
==========================================
+ Hits        10865    10888      +23     
- Misses        881      897      +16     
Flag Coverage Δ
cpp 92.38% <100.00%> (-0.12%) ⬇️
python_and_cython 92.38% <100.00%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@godlygeek
Copy link
Copy Markdown
Contributor Author

Specifically, the full failure mode here is:

  • The user starts Memray tracking
  • The user does import greenlet
  • The greenlet module does from ._greenlet import ...
  • The interpreter dlopen's the _greenlet shared library
  • Memray's dlopen hook fires and sets a flag to tell our Python profile function to try to install our greenlet trace function the next time a Python instruction is run
  • The greenlet._greenlet of greenlet 3.4.0 does a PyImport_ImportModule("atexit") from it's initialization function
  • Because atexit is written in Python, our Python profile function runs
  • We try to access sys.modules["greenlet._greenlet"] but fail because it's not yet added to sys.modules
  • We fall back to using sys.modules["greenlet"] for greenlet 0.x compatibility
  • We try to access greenlet.settrace, which fails because it hasn't yet been imported by greenlet from greenlet._greenlet

The PyImport_ImportModule("atexit") was reverted in greenlet 3.5.0, but removing the fallback to import greenlet.settrace when greenlet._greenlet.settrace can't be imported fixes the issue for greenlet 3.4.0 and hardens us against the possibility of the greenlet module running other Python code in the future, or allowing a context switch to another thread (what we were doing was probably broken in free-threading builds as well).

Copy link
Copy Markdown
Contributor

@lkollar lkollar left a comment

Choose a reason for hiding this comment

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

Would it be worth mentioning that in the news fragment that this drops greenlet support before 1.0? LGTM otherwise.

Our attempts to handle ancient versions of greenlet are causing us to
try to import the `settrace` function from `greenlet` rather than from
`greenlet._greenlet`, which is failing because we're trying to import it
before greenlet's own `__init__.py` has imported it.

Drop support for greenlet versions older than 1.0, which is more than
5 years out of date at this point, to fix this issue.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
@godlygeek godlygeek force-pushed the fix_greenlet_handling branch from cab63e9 to f739785 Compare May 10, 2026 19:44
@godlygeek
Copy link
Copy Markdown
Contributor Author

Would it be worth mentioning that in the news fragment that this drops greenlet support before 1.0? LGTM otherwise.

Good point, done.

@godlygeek godlygeek enabled auto-merge (rebase) May 10, 2026 19:47
@godlygeek godlygeek merged commit 4a99614 into bloomberg:main May 10, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants