Repository navigation
Conversation
`sendRaw` writes with `Darwin.send(fd, …, 0)`, and neither the dialled descriptor nor the accepted one carried `SO_NOSIGPIPE`. A write to a peer that has reset the connection therefore raised `SIGPIPE`, whose default action ends the process — and nothing in the app, the Catalyst helper or the injected payload installs a handler for it. Both ends needed it, for opposite reasons. The app writes to an injected target that can die mid-request; the payload writes to an app that can quit, or that closes the connection with a reply still in flight, which answers with a reset. `stop()` reaches the same place from inside the process: its `shutdown(SHUT_RDWR)` wakes a blocked `send`, and what it returned was the signal rather than `EPIPE`. That path arrived with the 2026-04-10 review's M5.4, which added the `shutdown()` on the premise that the send would fail with an error. Set on the accepted descriptor explicitly rather than relying on the listening socket, which does not pass the option on. Swept the rest of the repository: the `send(2)` in `sendRaw` is the only raw socket write outside `RuntimeViewerCommandLine`, whose `UnixDomainSocket` has set this option on both its socket and its accepted descriptors since it was written — the comment there names this exact hazard, and this transport was simply never given the same treatment. The other transports go through `NWConnection` and never call `send(2)`. `RuntimeLocalSocketBrokenPipeTests` covers both halves: that the option reads back off a connected pair, and that a send to a peer which reset the connection throws `.sendFailed(errno: EPIPE)` while raising no `SIGPIPE`. The second test counts delivery attempts through a kqueue `EVFILT_SIGNAL` filter with the signal ignored, so the attempt it exists to catch cannot take the test run down with it. Measured before the fix: the option read back 0 on both ends and the signal was raised once.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The regression test must report signal-observation errors rather than treating them as successful zero-signal measurements.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Prevents broken-pipe writes in the local TCP transport from terminating Runtime Viewer or injected host processes.
Changes:
- Applies
SO_NOSIGPIPEto connecting and accepted sockets. - Adds option-readback and reset-peer regression tests.
- Documents the transport’s signal-suppression behavior.
| File | Description |
|---|---|
| RuntimeViewerCore/Tests/RuntimeViewerCommunicationTests/RuntimeLocalSocketConnectionTests.swift | Adds broken-pipe regression tests and signal-observation helpers. |
| RuntimeViewerCore/Sources/RuntimeViewerCommunication/Connections/RuntimeLocalSocketConnection.swift | Configures signal suppression on both connection paths. |
| Documentations/CommunicationAndEngineArchitecture.md | Explains suppression requirements and regression coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+911
to
+912
| guard kevent(kqueueFileDescriptor, nil, 0, &event, 1, &noWait) == 1 else { return 0 } | ||
| return event.data |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
RuntimeLocalSocketConnection.sendRawwrites withDarwin.send(fd, …, 0), andneither the dialled descriptor nor the accepted one carried
SO_NOSIGPIPE. Awrite to a peer that has reset the connection therefore raised
SIGPIPE, whosedefault action ends the process — and nothing in the app, the Catalyst helper,
the CLI host or the injected payload installs a handler for it.
XNU's
senditis explicit about the rule: whensendwould returnEPIPEandthe socket has neither
SOF_NOSIGPIPEnorMSG_NOSIGNAL, it callspsignal(p, SIGPIPE).Why it is reachable
EPIPEfollowsSS_CANTSENDMORE, which a received RST sets. Three ways in:side closes with unread data and the peer's next write is into a reset
connection — that peer is an injected process, so the process that dies is
the host of someone else's payload;
stop()callsshutdown(SHUT_RDWR), which wakes asendalready blocked inthe write queue.
The fix
One
setsockoptper descriptor, at both the connect site and the accept site —the same thing
RuntimeViewerCommandLine'sUnixDomainSocketalready does,whose comment says why: a write to a vanished peer must fail with
EPIPErather than kill the process. That comment was there and the rule had simply
never been applied to this transport.
Tests
RuntimeLocalSocketBrokenPipeTests, two cases: that the option reads backnon-zero on both ends of a real connection, and that a send to a peer which
reset the connection throws
EPIPEwhile a signal observer counts zerodeliveries. Measured before the fix — the option read back 0 on both ends and
the signal was raised once.
Full
RuntimeViewerCommunicationTestson top: 174 tests, 0 failures.Notes
mainat all —the pinned SwiftyXPC 0.5.104 fails with
stored properties cannot be marked unavailable with '@available', unrelated to this change.feature/jailbroken-ios-injectioncarries the same fix in its own shape,because that branch has a shared
configureSocketOptionshelper thatmaindoes not. The two will meet when
mainmerges intonext; collapse them tothe shared helper there.