-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(python): Raise an error when users try to use Polars API in a fork()-without-execve() child #19149
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19149 +/- ##
==========================================
- Coverage 79.78% 79.76% -0.03%
==========================================
Files 1531 1531
Lines 208445 208455 +10
Branches 2913 2916 +3
==========================================
- Hits 166301 166264 -37
- Misses 41593 41639 +46
- Partials 551 552 +1 ☔ View full report in Codecov by Sentry. |
# fork() breaks Polars thread pool. Instead of silently hanging when users do | ||
# this, e.g. by using multiprocessing's footgun default setting on Linux, warn | ||
# them instead: | ||
def __install_postfork_hook() -> None: |
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.
Nice fix. Any idea of the startup cost of this?
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.
I suspect it's not meaningfully slower, but I'll measure it tomorrow.
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.
This appears to add 3 or 4 µs to import polars
. This doesn't seem like a problem to me.
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.
Alright. Thanks @itamarst. Nice that we can catch those deadlocks now.
… in a fork()-without-execve() child (pola-rs#19149)" This reverts commit 48bc09b.
This is an alternative implementation for #5342, which was reverted due to side-effects.
This implementation should not affect any non-Polars using code. It won't catch all cases of people using Polars in a fork()-without-execve() child, but it will catch many of them.