Fix macOS crash and reconnect loop when a connection is refused (#814)

[romainhedouin]

Fixes #814.

Summary

On macOS 27, Syncplay crashes with a segfault in QCoreApplication::sendEvent when it connects to a server whose IPv6 address refuses connections, such as syncplay.pl. That is why it appears to crash when clicking "Store configuration and run Syncplay": that click is the first connection attempt. Avoiding the crash alone is not enough, because the client then loops on "Connection with server lost, attempting to reconnect". Both problems come from the vendored Twisted Qt reactor (qt5reactor.py) interacting with Qt's CFSocket-based QSocketNotifier on macOS, so this PR fixes both.

Root causes

1. Use-after-free crash. A refused connection is both readable and writable. Qt's macOS dispatcher delivers the read and write callbacks for the socket back-to-back. Handling the first one makes Twisted disconnect the socket, which calls removeReader/removeWriter and unregisters both notifiers. That frees Qt's per-socket data while the second callback is still pending, and the second callback then uses it. This matches the crash reports: __CFSocketPerformV0 → QtCore → QCoreApplication::sendEvent, at either a null-ish address (0x58) or a freed one. It is a race, so it is intermittent (about 10 in 12 runs on an unmodified checkout against a closed port).

2. Refused connections look successful. Once a write QSocketNotifier is registered on a connecting socket, Qt's CFSocket layer consumes the socket's pending SO_ERROR. Twisted's doConnect() then reads SO_ERROR == 0, calls connect_ex() again, gets EISCONN, and decides the connection succeeded. A raw-socket check on macOS 27.0.1 shows the underlying behaviour: after a refused non-blocking connect, the first SO_ERROR read gives ECONNREFUSED, and later reads give 0 with connect_ex returning EISCONN. With only a write notifier registered, SO_ERROR is already 0 after Qt has dispatched events. Because Syncplay races IPv6 against IPv4, the falsely "connected" IPv6 attempt wins, sends startTLS, reads EOF, and the client reconnects forever. Before this PR the crash happened first, so this was never visible.

Changes (syncplay/vendor/qt5reactor.py only)

• macOS only: TwistedSocketNotifier.read() / write() now only disable their own notifier inside Qt's callback and process the event just afterwards via QTimer.singleShot(0, ...). Unregistration therefore never happens while a sibling callback for the same socket is still pending, and it still happens synchronously and before Twisted closes the fd. (I tried deferring the unregistration itself first. That breaks normal connections because the fd number can be reused before the late unregister runs, so this PR does not do that.)
• macOS only: before processing an event for a socket that is still connecting, _connectionRefused() checks whether the connection really exists. If SO_ERROR is set or the socket has no peer (getpeername() raises), it calls failIfNotConnected() with the real errno, or ECONNREFUSED when the error was already consumed. Twisted then reports ConnectionRefusedError as it normally would.
• Other platforms run the original code path unchanged.

Testing

Environment: macOS 27.0.1, Apple Silicon, Python 3.12, PySide6 6.11.2 (the same Qt as the 1.7.6 app bundle that crashes), Twisted 26.4.0. Everything below was run against the final code. "Before" means an unmodified checkout.

Scenario	Before	After
Bare reactor, refused TCP connect (IPv4 and IPv6, repeated)	segfault in about 10 of 12 runs	15 of 15 clean
Reported result of a refused connect	connectionMade (wrong)	ConnectionRefusedError, 10 of 10 each for 127.0.0.1, ::1 and syncplay.pl's IPv6 address
Full client: click Store with an unreachable server	segfault, 6 of 6	no crash, 4 of 4
Full client against syncplay.pl	segfault	TLS 1.3 established, Hello handshake, about 17 state updates, 0 "connection lost", 3 of 3
Normal accepted connection with data exchange and clean disconnect (local server)	works	works, output identical

I have not run this on Windows or Linux. Both fixes are guarded by sys.platform == 'darwin', so those platforms keep the existing code path, but I'd appreciate a review from someone who can.

Notes

• The getpeername() check is a workaround for losing SO_ERROR, so it can only report ECONNREFUSED when the original error was consumed. Failures such as a timeout or an unreachable host surface as "connection refused" in that case. That is still correct as "failed", and the client retries as it would anyway.
• The real fix would be in Qt or macOS. If Qt's dispatcher stops consuming SO_ERROR or tolerates unregistering from within a callback, this workaround becomes unnecessary and can be dropped.
• This is a vendored file that was already carrying local patches, so I kept the diff small and commented.

🤖 Generated with Claude Code

[romainhedouin]

@Et0h FYI Syncplay is currently unusable on macOS without this fix

[Et0h]

Thanks for the wok on this.

@Et0h FYI Syncplay is currently unusable on macOS without this fix

Is this only a problem when connecting to the default Syncplay public server port or for all ports?

[romainhedouin]

Thanks for this very useful project :)

It affects all ports
Closed local ports all segfault or falsely claim to have connected
Real syncplay.pl 8995-8999 segfault too or falsely win then drop
Local IPv4-only localhost server ports all crash or falsely claim to connect

Keep in mind that I've never worked on this project before and networking is not my strength, so take my code with a grain of salt
What I can say for sure is that I watched a 2 hour movie with this fix so it must be halfway decent haha

[TheCrazyLex]

@albertosottile @Et0h @romainhedouin So I had the same crash on mac os 27 and installing this fix resolved it.

Also reviewed the code, and this is the correct approach to address this from my perspective

[xboxfanj]

This fixes the crash for me as well on macOS 26.7 and the change looks good.

[albertosottile]

LGTM in general, though I am not an expert on the matter. Just a minor question. Additionally, I wonder whether we should submit a bug ticket to PySide6.

[Et0h]

Cheers @romainhedouin for your work on this, and to everyone who tested and reviewed the fix.

The code looks fine to me, but I've not been able to test it myself so it's good to see from everyone's testing that it works on macOS 26 and 27 and appears to resolve the issue without any apparent problems.

Given that we're using a vendored Qt reactor, I don't see upstream acceptance as a requirement for merging, but I agree with @albertosottile that consideration should be given to reporting the matter upstream to Qt/PySide6. What are your thoughts on that, @romainhedouin?

[romainhedouin]

I just dug into it and Qt fixed it in their 6.12.0 version, which was published two days after I created this PR
A Syncplay build created now with the old Syncplay code should therefore be expected to work
Here is their commit that fixes it: qt/qtbase@52f1ba174