Skip to content

test(run): re-sample main's signal mask past the posix_spawn window (LAB-11681) - #41

Merged
27Bslash6 merged 1 commit into
mainfrom
lab-11681-sigblk-resample
Oct 30, 2026
Merged

27Bslash6 merged 1 commit into
mainfrom
lab-11681-sigblk-resample

Conversation

@27Bslash6

Copy link
Copy Markdown
Contributor

helper_threads_block_the_handled_signals failed intermittently under CPU load with "the main thread blocks a handled signal". The race is in the test, not in ck: std::process::Command::spawn uses glibc's posix_spawn, which blocks every signal on the calling thread until the child execs. The command can record its run before ck's main thread is scheduled again, so the test's single read of main's SigBlk could land inside that window. A signal sent then stays pending and is delivered when glibc restores the mask, so nothing is lost.

Change (tests/run.rs, test only)

  • Re-sample main's SigBlk every 5 ms for up to 2 s, and fail only if a handled signal (HUP, INT, QUIT, TERM) stays blocked.
  • The assertion message prints the observed mask, e.g. SigBlk 0000000000004000.
  • The command sleeps 5 s instead of 2 s, so ck is still running for the whole re-sample.
  • The helper-thread half of the assertion is unchanged.

Verification

60 isolated runs of the one test, pinned to one CPU beside three busy loops:

Test ck Pass Fail
unmodified unmodified 48 12
this PR unmodified 60 0
this PR mutant: main keeps SIGTERM blocked 0 60

Checks (local, as CI runs them)

  • cargo fmt --all -- --check: pass
  • cargo clippy --locked --all-targets -- -D warnings: pass
  • cargo test --locked: lib 22, readme 1, run 45, saas 17 passed; bench 1 ignored; 0 failed

No docs change: test-only, no user-facing behaviour.

Closes LAB-11681

Spawning the command through glibc's posix_spawn blocks every signal on
the calling thread until the child execs. The command can record its run
before ck's main thread is scheduled again, so a single read of main's
SigBlk under CPU load caught that transient mask and failed.

Re-sample main's mask for up to 2 s and fail only if a handled signal
stays blocked; the message now prints the observed mask. The command
sleeps 5 s so ck outlives the re-sample.
@kodus-27b

kodus-27b Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

@27Bslash6
27Bslash6 merged commit a782274 into main Oct 30, 2026
5 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.

1 participant