Skip to content

fix: Preserve interruptible syscalls when registering the SIGINT handler - #28805

Open
skmendez wants to merge 1 commit into
pola-rs:mainfrom
skmendez:fix-sigint-handler
Open

fix: Preserve interruptible syscalls when registering the SIGINT handler#28805
skmendez wants to merge 1 commit into
pola-rs:mainfrom
skmendez:fix-sigint-handler

Conversation

@skmendez

@skmendez skmendez commented Aug 13, 2026

Copy link
Copy Markdown

Resolves #21739. Before registering the SIGINT handler, we check the current state of SA_RESTART, and if it's not already set, we disable it after running signal_hook::low_level::register. The maintainer of signal-hook does not want to support this feature (see discourse at vorner/signal-hook#153), so options for resolving this are either to reimplement cross-platform signal handling here or to just do this sigaction workaround.

Copying the minimal example from #21739 (comment):

import polars

# you can now ctrl+c this!
input()

Note that originally I tried to add unit tests for this fix, but getting a unit test of signal handling behavior working consistently on high-contention CI machines proved fairly difficult, and the original KeyboardInterrupt logic is already also untested AFAICT, so I dropped these tests. Happy to try to readd them if there's interest though.

AI usage disclosure:
I used codex to write the fix, based on the workaround discussed in vorner/signal-hook#153. This commit message and all code comments (including the SAFETY comments) were written by me. I have manually reviewed all changes and I believe they are correct.

First-time contributors must adhere to the following rules, or your PR(s) will be closed:

  • You must post a screenshot of you successfully running the test suite (make test),
    locally on your machine (not the CI). The screenshot must show your terminal window
    borders clearly, it must not be cropped to only show text.
image

@github-actions github-actions Bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars first-contribution First contribution by user labels Aug 13, 2026
@skmendez
skmendez force-pushed the fix-sigint-handler branch from 70211c2 to d50f1a8 Compare August 13, 2026 17:54
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.51724% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.50%. Comparing base (df59905) to head (afc4eda).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
crates/polars-error/src/abort.rs 65.51% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #28805      +/-   ##
==========================================
+ Coverage   81.27%   81.50%   +0.23%     
==========================================
  Files        1887     1887              
  Lines      266898   266916      +18     
  Branches     3238     3238              
==========================================
+ Hits       216908   217561     +653     
+ Misses      49163    48528     -635     
  Partials      827      827              

☔ View full report in Codecov by Harness.
📢 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.

@skmendez
skmendez force-pushed the fix-sigint-handler branch 4 times, most recently from c26bd19 to b191660 Compare August 17, 2026 16:00
@skmendez
skmendez force-pushed the fix-sigint-handler branch from b191660 to afc4eda Compare August 17, 2026 16:39
@skmendez
skmendez marked this pull request as ready for review August 17, 2026 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first-contribution First contribution by user fix Bug fix python Related to Python Polars rust Related to Rust Polars

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Polars' SIGINT handler sets SA_RESTART flag, preventing syscalls from being interrupted

2 participants