Fix intermittent segfault in the Becker-port TCP server - #8
Open
peacedudes wants to merge 2 commits into
Open
Conversation
DriveWireHost is written against the main queue: accepted tcp-listen connections append guest input and refresh channel status via DispatchQueue.main, and its idle watchdog is a Timer, which needs a running run loop. The Becker-port listener delivered its callbacks on a private queue instead, so host.send(data:) read the same dictionaries the main queue was mutating. Thread Sanitizer flags the race; in practice drivewire-cli segfaults inside pollVirtualSerial(). Deliver every callback on the main queue -- the contract DriveWireTCPDriver already keeps -- which also keeps this driver's own connection property on a single queue. Co-Authored-By: Claude Fable 5 <[email protected]>
…le 0x00 Add DriveWireTCPServerDriverTests: a QueueRecorder stands in as the host's delegate and asserts guest traffic reaches DriveWireHost on the main queue, where the host's own virtual-serial paths run. Against the previous driver it fails with the host driven on DriveWireTCPServer; the fix turns it green. A sanity test proves the listener answers OP_TIME at all, so a harness fault cannot masquerade as the regression. Also correct testDWINIT: it still expected 0x00 from OP_DWINIT, which deliberately answers 0xFF -- its own comment records that a zero makes NitrOS-9 treat the host as a DW3 server and disable every DW4 extension. Co-Authored-By: Claude Fable 5 <[email protected]>
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
1 participant
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.
drivewire-cli --tcp-portcrashes intermittently while serving an XRoar/NitrOS-9 guest:EXC_BAD_ACCESSinDictionary._Variant.lookup, underDriveWireHost.pollVirtualSerial(). We hit it five times in one day driving automated NitrOS-9 sessions throughinetdvirtual-serial channels.Cause.
DriveWireHostis effectively a main-queue object: acceptedtcp listenconnections append guest input and refresh channel status viaDispatchQueue.main.async, and the idle watchdog is a run-loopTimer.DriveWireTCPServerDriver, though, delivered its Network callbacks on a private queue, sohost.send(data:)read the same dictionaries the main queue was mutating.DriveWireTCPDriveralready hops to the main queue before callinghost.send(data:)for exactly this reason — the Becker-port listener was the one path that didn't.Fix. One line: the driver's callback queue becomes
DispatchQueue.main, so the listener, the guest connection, andhost.send(data:)all run where the host expects. The CLI'sRunLoop.current.run()services the main queue.Evidence.
drivewire-cliagainst a live XRoar/NitrOS-9 boot plus repeated inetd login sessions: 2 distinct Swift access races before (both reachingpollVirtualSerialstate), 0 after.testGuestTrafficReachesTheHostOnTheMainQueuefails onmain— "The host was driven on DriveWireTCPServer instead of the main queue" — and passes with the fix. A sanity test (testOpTimeAnswersOverTheListener) proves the listener answers at all, so a harness fault can't masquerade as the regression.DriveWireTestssuite passes, three consecutive runs.Also in the tests commit
DriveWireTCPServerDriver.swiftis added to theDriveWireTeststarget so the new test can reach it.testDWINITexpected0x00, butOP_DWINITdeliberately answers non-zero (0xFF) — its comment notes that a zero makes the NitrOS-9 driver treat the host as a DW3 server and disable the DW4 extensions — so that test currently fails onmain. Updated the expectation to0xFF. Happy to drop this hunk if you'd rather handle it separately.🤖 Generated with Claude Code