Repository navigation
Sockets: add the Qt ServerSocket and the loopback test harness - #1787
Merged
Merged
Conversation
pa-devbot
force-pushed
the
sockets-server-loopback
branch
2 times, most recently
from
October 31, 2026 22:58
936fa57 to
8f44f29
Compare
pa-devbot
changed the title
pa-devbot
force-pushed
the
sockets-server-loopback
branch
from
November 1, 2026 02:44
82d5b0f to
544b9fb
Compare
Implements ServerSocket_Qt (a QTcpServer on the Qt event thread pool, mirroring ClientSocket_Qt; the implementation lives in ServerSocket_Qt.cpp, following the split of the client backends) and adds loopback tests that combine a ClientSocket and a ServerSocket against each other on 127.0.0.1. The test logic is written against the abstract AbstractClientSocket/AbstractServerSocket classes and is instantiated per backend; this PR instantiates the Qt backend on all platforms. The POSIX and WinSocket backends (and their test instantiations) land in separate PRs on top of this one, so that every platform runs 2 of the 3 backends and CI as a whole covers all 3. Scenarios: echo round trip, zero-length sends, a 1 MiB transfer with integrity check, rapid reconnect cycles, a send/close race, rejection of a second client, and a port-in-use start failure. Also documents that ServerSocket::close_connection() is safe to call from within a listener callback (close() is not: it joins the worker threads).
pa-devbot
force-pushed
the
sockets-server-loopback
branch
from
November 1, 2026 04:53
544b9fb to
20140b7
Compare
AGENTS.md forbids using std::mutex/std::condition_variable directly; use the PokemonAutomation::Mutex/ConditionVariable wrappers from Common/Cpp/Concurrency instead. Migrate the Qt server's completion structs and the test harness's Waiter/rng lock.
The method bodies kept their in-class 8-space indentation from the monolithic header; un-dedent them to file scope (and drop the stray blank lines the split left behind).
This was referenced Nov 1, 2026
The 5 s completion waits in send()/close_connection() were only safe while the queued call ran within the window: the completion objects were stack-local, so if the wait expired and the event thread delivered the call later, on_send/on_close_connection wrote through a dead stack pointer (stack-use-after-return; reproduced under ASAN by stalling the event thread in on_receive_data for 6 s). The signals now pass QSharedPointer<SendData>/QSharedPointer<CloseCompletion> by value: the queued call's own copy keeps the payload alive for as long as the call is pending or running (no UAF after a timed-out caller), and if close() destroys the QTcpServer first, Qt discards the queued call and the caller's copy is the last reference (no leak). The 5 s bounds remain as plain safety valves. Also add the regression test 'Sockets (Qt) - Completion lifetime under event-thread stall', which stalls the event thread inside on_receive_data, verifies close_connection() times out at ~5 s instead of hanging, and that the late handler still restores LISTENING. Takes ~12 s to run.
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.
Adds the Qt
ServerSocketbackend plus the loopback test harness.This is the minimal first piece of the server-socket work, split out so
the backends can be reviewed/merged independently (see the two follow-on
PRs that are based on this one).
What's here
ServerSocket_Qt: aQTcpServeron the Qt event thread pool,mirroring
ClientSocket_Qt. One connection at a time; extra clientsare rejected via
on_connect_rejected.Sockets_Tests.cpp/.h: loopback tests that combine aClientSocketand a
ServerSocketon 127.0.0.1. The test logic is written againstthe abstract classes and instantiated per backend; this PR enables the
Qt backend on all platforms.
ServerSocket::close_connection()is safe to call froma listener callback (
close()is not, it joins the worker threads).Scenarios (each as its own test): echo round trip, zero-length send,
1 MiB transfer with integrity check, rapid reconnects, send/close race,
reject a second client, port-in-use start failure.
Not in this PR (land in the PRs based on it):
Result: every platform runs Qt here, and once the follow-ons land,
every platform runs 2 of the 3 backends with CI covering all 3.
Note (rebuild on current main): the implementation now lives in
ServerSocket_Qt.cpp, mirroring theClientSocket_Qtheader/.cpp split (#1795).Update (AGENTS.md + formatting):
std::mutex/std::condition_variableto thePokemonAutomation::Mutex/ConditionVariablewrappers(
Common/Cpp/Concurrency/) inServerSocket_Qt.hand the testharness's
Waiter/port-rng lock.ClientSocket_Qt.h(alreadymerged) had the same one-member violation in its
SendDatastruct; that follow-up migration is included here too.
ServerSocket_Qt.cppindentation left over from theheader/.cpp split (method bodies had kept their in-class 8-space
indent).
Update (completion lifetime): the
send()/close_connection()5 s waits are now safe on expiry. The completions were stack-local,
so if the wait timed out and the event thread delivered the queued
call afterwards, the handler wrote through a dead stack pointer
(stack-use-after-return — reproduced under ASAN by stalling the
event thread in
on_receive_data). The signals now passQSharedPointer<SendData>/QSharedPointer<CloseCompletion>byvalue: the queued call's copy keeps the payload alive for as long as
the call is pending or running, and if
close()destroys theQTcpServerfirst, Qt discards the queued call and the caller'scopy is the last reference (no UAF, no leak). Added a regression
test, "Sockets (Qt) - Completion lifetime under event-thread stall"
(~12 s): it stalls the event thread in an
on_receive_datacallbackfor 6 s, checks
close_connection()times out at ~5 s instead ofhanging, and that the late handler still restores
LISTENING.