bloomberg / memray

Memray is a memory profiler for Python
https://bloomberg.github.io/memray/
Apache License 2.0
13.36k stars 397 forks source link

Fix a regression in handling processes that fork #645

Closed godlygeek closed 4 months ago

godlygeek commented 4 months ago

We switched from pthread_atfork handlers to os.register_at_fork ones in order to support Python 3.13, where a lock that our child fork handler needs to acquire will not have been reinitialized in the child process at the time when pthread_atfork handlers run but will be reinitialized by the time os.register_at_fork handlers run.

Unfortunately, this means that if a process forks without calling the before handler registered with os.register_at_fork (as happens when using the "spawn" start method for multiprocessing, the default on macOS), then our hooks are unexpectedly still active from the time the process forks until the time that it execs another process image.

We can work around this by using a mix of both pthread_atfork and os.register_at_fork handlers: we suspend tracking in a pthread_atfork prepare handler, resume it in a pthread_atfork handler in the parent process, and (if --follow-fork mode is used) we reinitialize it in the child process from an os.register_at_fork child handler, after the interpreter's locks have been reinitialized.

Closes: #642

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.06%. Comparing base (41248ed) to head (41ce62f). Report is 103 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #645 +/- ## ========================================== + Coverage 92.55% 93.06% +0.51% ========================================== Files 91 94 +3 Lines 11304 11415 +111 Branches 1581 2092 +511 ========================================== + Hits 10462 10623 +161 + Misses 837 792 -45 + Partials 5 0 -5 ``` | [Flag](https://app.codecov.io/gh/bloomberg/memray/pull/645/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg) | Coverage Δ | | |---|---|---| | [cpp](https://app.codecov.io/gh/bloomberg/memray/pull/645/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg) | `93.06% <100.00%> (+7.12%)` | :arrow_up: | | [python_and_cython](https://app.codecov.io/gh/bloomberg/memray/pull/645/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg) | `93.06% <100.00%> (-2.66%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.