Skip to content

fix(examples): discard a SIGALRM still pending as _deadline puts the handler back (#2090) - #2091

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2090-deadline-pending-sigalrm
Oct 23, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2090-deadline-pending-sigalrm

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — bug fixes

Description of your pull request and other information

Closes #2090. examples/generators/options.py's _deadline.__exit__ cancels with alarm(0) and then restores the previous handler. On GraalPy a SIGALRM that fired just before the cancel can still be pending at that point. If the handler being restored is SIG_DFL, it then kills the process. This is the defect #2084 fixed in tests/_support.py (403117c98).

  • Fix: set SIG_IGN between the cancel and the restore, which discards a pending signal.
  • Test: test_a_firing_still_pending_as_the_deadline_cancels_is_discarded holds the signal pending with pthread_sigmask, so the ordering is forced. It fails without the fix ('handled': [14] != []) and skips where pthread_sigmask is missing, GraalPy included.
  • Samples: make_samples.py gives byte-identical examples/captures/ with and without the fix.
  • Results: the generator, option and round-trip test files pass on CPython 3.14.9.

…handler back (#2090)

_deadline.__exit__ cancelled the alarm and restored the previous handler
directly, so on GraalPy a SIGALRM that fired just before the cancel could be
delivered to SIG_DFL afterwards and end the worker, as in #2084. Ignoring the
signal first discards it. The new test holds it pending with pthread_sigmask.
@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: running A cross-review is in flight against the current head - no verdict yet test Pull requests that add or correct tests (test: subject prefix) labels Oct 23, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 34af47986 (Sonnet review).

  • Fix: the order is cancel → SIG_IGN → restore. _deadline.__exit__ never re-arms an enclosing deadline, and expire returns early once settled, so the ignore window drops nothing that should be delivered. The armed guard still covers the path where no alarm was armed.
  • Test: with options.py reverted to 403117c98, the new test fails on its assertEqual and the other 14 still pass. With the fix, all 15 pass. Its finally unblocks the signal and confirms no timer is left.
  • Coverage: git grep finds no other cancel-then-restore site besides tests/_support.py (fixed in test: a GraalPy xdist worker died during a SIGALRM TimeLimitTests case #2084) and this one.
  • Not re-checked by the reviewer: the byte-identical captures. The author compared the sha256 of all examples/captures/ with and without the fix.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 23, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.54% (unit tier, Python 3.15, 34af47986, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19599 880 2618 866 92.12%
pcapkit/corekit 2565 71 812 37 96.56%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 2930 23 1022 25 98.79%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17414 219 4574 185 98.14%
pcapkit/toolkit 869 36 298 10 95.20%
pcapkit/utilities 473 7 136 5 98.03%
pcapkit/vendor 4432 1988 1018 167 51.87%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at 34af47986.

Merging it closes #2090.

@JarryShaw
JarryShaw merged commit afc286d into main Oct 23, 2026
65 checks passed
@JarryShaw
JarryShaw deleted the fix/2090-deadline-pending-sigalrm branch October 23, 2026 17:22
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 23, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 23, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

test: _Deadline restores its handler with a SIGALRM still pending, as #2084 did

1 participant