镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(os): avoid recursive signal cancellation lock - #908

Open
stephenlclarke wants to merge 1 commit into
apple:mainfrom
stephenlclarke:upstream/signal-cancellation-deadlock
Open

stephenlclarke wants to merge 1 commit into
apple:mainfrom
stephenlclarke:upstream/signal-cancellation-deadlock

Conversation

@stephenlclarke

Copy link
Copy Markdown
Contributor

Summary

Avoid recursively acquiring AsyncSignalHandler's state mutex during cancellation.

cancel() previously called AsyncStream.Continuation.finish() while holding the lock. Finishing invokes the continuation's onTermination callback, which calls cancel() again and can deadlock on the same non-recursive mutex.

Drain the continuations and dispatch sources under the lock, clear the shared state, then finish and cancel those resources after releasing the lock. This keeps repeated cancellation idempotent while removing the recursive lock path.

Testing

swift test --filter AsyncSignalHandlerTests

Result: 1 focused regression passed. It creates multiple signal streams, cancels them, verifies both finish, and then cancels again.

Compatibility

No public API changes. Signal sources and streams are still cancelled by the same call; only the lock boundary changes.

@stephenlclarke
stephenlclarke force-pushed the upstream/signal-cancellation-deadlock branch from b4cc9e3 to 471b580 Compare September 28, 2026 08:03
struct AsyncSignalHandlerTests {
@Test
func cancelFinishesStreamsWithoutRecursivelyLocking() async {
let handler = AsyncSignalHandler.create(notify: [SIGUSR1])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think installing a signal handler in the test process could cause issues/flakes?

@stephenlclarke stephenlclarke Oct 19, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased onto current upstream 6adea5663427b4068108e9268e6125fc139b6045 and updated the regression in 19eb7f900324b7999678f0174b1af4474169b45c. The test now uses notify: []: it exercises both stream termination callbacks and repeated cancellation without installing a process-wide signal disposition in the shared test runner.

A standalone executable compiled from the exact production file reproduces the original failure after entering cancel() and before returning; the updated implementation returns, drains both streams, and tolerates repeated cancellation. This is isolated / focused cancellation evidence.

@stephenlclarke
stephenlclarke force-pushed the upstream/signal-cancellation-deadlock branch 2 times, most recently from 19eb7f9 to afa1fc6 Compare October 21, 2026 17:30
@stephenlclarke
stephenlclarke force-pushed the upstream/signal-cancellation-deadlock branch from afa1fc6 to abddf8c Compare October 28, 2026 11:37

This branch has not been deployed

No deployments
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.

2 participants