Skip to content

Fix PinOne swallowing connect failures (zombie updater thread)#56

Merged
jsm174 merged 1 commit into
vpinball:masterfrom
dynajoe:fix/pinone-connect-throw
Jul 6, 2026
Merged

Fix PinOne swallowing connect failures (zombie updater thread)#56
jsm174 merged 1 commit into
vpinball:masterfrom
dynajoe:fix/pinone-connect-throw

Conversation

@dynajoe

@dynajoe dynajoe commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Fix

ConnectToController logged a warning and returned on connect/create-server failure, and the catch swallowed without rethrow. PinOne.cs throws on all three paths. Throw on failure and rethrow in the catch to match.

Why it matters

The base updater thread (OutputControllerCompleteBase) is written to quit cleanly when ConnectToController throws. Because the port swallowed the failure, a failed connect was treated as success — the thread logged "connected" and then spun against an unconnected port (write-fail → catch → disconnect / sleep / reconnect, forever) instead of quitting. Part of the PinOne-on-Linux shutdown-robustness family alongside the SO_RCVTIMEO teardown fix in #53, and the "updater thread did not quit / solenoid stuck on exit" symptoms discussed in #31.

Found by diffing against the DirectOutput C# source.

ConnectToController logged+returned on connect/create-server failure and the catch swallowed without rethrow; PinOne.cs throws on all three. The base updater thread is written to quit when ConnectToController throws -- so with the swallow a failed connect is treated as success and the thread spins against an unconnected port instead of quitting cleanly. Throw on failure and rethrow in the catch to match PinOne.cs.
@jsm174

jsm174 commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Hello. Thanks for the PR!

Converting this to draft as per the contributing rules. (I was behind in getting them added as was just done in Visual Pinball).

It's going to take time to review these. libdof is currently is use by several users with a variety of devices, and we have yet to receive issues about this.

Also this definitely needs thorough testing with the actual hardware.

@jsm174
jsm174 marked this pull request as draft July 5, 2026 18:08
@jsm174
jsm174 marked this pull request as ready for review July 6, 2026 21:32
@jsm174
jsm174 merged commit 74ac62d into vpinball:master Jul 6, 2026
13 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.

2 participants