[pa-devbot]

Follow-up to the review of Common/Cpp/Sockets (the Windows one that isn't used on Windows because "it has issues"). This fixes the ClientSocket_WinSocket implementation so it can be examined/tested.

What was broken

1. It didn't compile. connect() called ThreadPool::blocking_dispatch(), which no longer exists (the API is dispatch_now_blocking(); see Common/Cpp/Concurrency/ThreadPool.h). The file had been stale since the thread-pool API was renamed.

2. Connection resets were never detected. When the peer closed the connection, recv() == 0 entered an infinite 1 ms poll loop; on socket errors (e.g. WSAECONNRESET) the receive thread exited. Either way the state stayed CONNECTED and no callback fired, so the owner (sysbotbase) believed it was still connected forever.

3. close() didn't close the socket and left the state stuck at DESTRUCTING permanently, so the object could never be reused (one-shot).

4. Partial-send data loss. send() counted/advanced by the requested bytes (current) instead of the bytes actually sent (current_sent), so a partial send silently dropped the remainder and reported full success. (POSIX got this fixed in 4d3797d20.)

5. Double mutex unlock. The connect-failure paths called m_lock.unlock() on the raw mutex while a std::unique_lock on that same mutex was still active (UB / crash on the "connection refused" path).

6. Minor: WSAEINVAL in the connect loop was retried forever; the INVALID_SOCKET guard in send() was dead code because m_socket was const.

What the fix does

• New AbstractClientSocket::Listener::on_disconnect(error_message) callback (empty by default, so it's backward-compatible). Fired when an established connection is lost (server close or socket error). It is not fired by close().
• Connection loss now returns the socket to NOT_RUNNING, so connect() can be called again on the same object.
• close() now actually tears down: it waits for the receive thread to exit — including any in-flight listener callback — closes the socket, and returns the socket to NOT_RUNNING. This also closes the use-after-free window where the owner could destroy itself while the receive thread was still calling back into it (the remove_listener fix in Sockets: fix AbstractClientSocket::remove_listener() #1772 was the other half of that). It's idempotent, but must not be called from within a listener callback (it would wait on itself).
• send() now accounts for current_sent.
• connect() creates a fresh socket each time, since a socket whose previous connection ended cannot be reconnected. Socket creation failure reports through on_connect_finished.
• The connect-failure paths no longer double-unlock the mutex.
• m_socket is now std::atomic<SOCKET> (initialized INVALID_SOCKET in the constructor rather than eagerly creating the socket).

The sysbotbase connection overrides on_disconnect() to log the disconnect and update the status line, so a restart of sys-botbase is now visible instead of a zombie "Last Ack: Ns ago".

Not wired in: Common/Cpp/Sockets/ClientSocket.h still selects ClientSocket_Qt on Windows. Flip the alias there if you want to try this.

Testing

Can't build/run the Windows file on Linux, so it was verified with a mock winsock.h harness driving the real class + thread pool:

• 26 functional checks: connect (incl. non-blocking in-progress rounds), full + partial send accounting, receive, graceful-close → on_disconnect + NOT_RUNNING, send-after-disconnect returns 0, reconnect on the same object, RST → on_disconnect + NOT_RUNNING, idempotent close(), connect-refused → error callback with no crash (the old double-unlock path), and retry-after-refused.
• Stress: close() while a slow on_receive_data is in flight blocks until it finishes and no callback runs after close() returns; 200 rapid connect/disconnect cycles.

The Linux build (pa-build) and full command-line test suite (pa-test, 20,150 tests) pass with the AbstractClientSocket.h and sysbotbase changes.

[pa-devbot]

Two updates:

1. Sockets: add Listener::on_disconnect() hook #1780 (the on_disconnect hook) has merged into main, so I rebased this PR onto main - it now contains only the ClientSocket_WinSocket.h changes.
2. New commit: the WinSocket backend now calls WSAStartup itself (exactly once, before the first socket). Nothing in the program ever called it - the backend was only working because Qt initializes Winsock first. If this backend is used before any Qt networking call, the first socket() fails with WSA_NOT_INITIALIZED (10093). Re-verified with the mock-winsock functional + stress tests after the change.

[pa-devbot]

New commit (607b722) - fixes the intermittent WSA Error Code: 10022 you hit when rapidly resetting the connection:

Calling connect() again while a non-blocking connect is in progress is not safe on Windows: once the attempt fails, the second connect() returns WSAEINVAL (10022) instead of starting a new attempt. So the retry loop was masking the real failure behind 10022.

It now does the portable pattern instead: one connect() call, then select() for writability and getsockopt(SO_ERROR) for the result (same as the POSIX implementation). The real error is now reported: refused -> "Connection refused.", timeout -> "Connection timed out.", anything else -> its WSA code.

Expectation for your rapid-reset test: the first reconnect right after a reset may still fail - the dock is probably RSTing the new attempt while it is still tearing down the old connection (the new connect likely reuses the same 4-tuple, since the old one was RST-closed, not FIN-closed). But the status will now show the real reason (reset/refused) instead of 10022, and the retry ~100 ms later succeeds, as in your log.

Re-verified with the mock-winsock harness: added a scenario where the connect attempt settles as a failure mid-flight - the implementation reports the real error and never calls connect() a second time (the mock reproduces the Windows WSAEINVAL quirk, so the old loop would have reported 10022 on it). Functional + stress suites all pass.