Tomas/connection raii initial#413
Draft
ProfessorTom wants to merge 78 commits intodannagle:developmentfrom
Draft
Tomas/connection raii initial#413ProfessorTom wants to merge 78 commits intodannagle:developmentfrom
ProfessorTom wants to merge 78 commits intodannagle:developmentfrom
Conversation
…tart unit testing
- New ctor: TCPThread(host, port, initialPacket, parent) - Creates QSslSocket early - Connects connected/errorOccurred/stateChanged signals - Stores host/port for run() - Sets m_managedByConnection = true
- [[nodiscard]] bool isValid() const - Checks clientConnection null, error codes, socket state - Adds detailed qDebug/qWarning logging for failure reasons
- Add null check for clientConnection before calling close() - Add waitForDisconnected(1000) for clean shutdown when socket exists - Log warning if called with null socket - Prevents potential segfault in dtor when socket not initialized
- Add if (clientConnection) guard before close() - Only call waitForDisconnected() if socket is not UnconnectedState or ClosingState - Log when wait is skipped or socket is null - Eliminates Qt warning about waitForDisconnected in UnconnectedState - Prevents potential null dereference in Connection dtor
…safe cleanup - Update ctor to pass host/port/initialPacket - Add public start() method with isValid() check - Auto-start in ctor - Add send(), isConnected(), isSecure() - Forward key TCPThread signals - Add m_threadStarted flag for safe dtor cleanup
- Add testThreadStartsAndStops() to verify no crash on start/dtor - Give thread time to start with QTest::qWait(500) - Check isConnected() (with fallback for Connecting state)
…plication - Link packet.cpp, tcpthread.cpp, sendpacketbutton.cpp - Add Qt6::Network for QSslSocket/QTcpSocket - Switch to per test class QApplication isolation with runGuiTest/runNonGuiTest
…bleWaitForReadyRead()` calls
…an be adjusted in a subclass
Ensure run() uses the correct address/port passed to the managed client constructor, preventing HostNotFoundError when sendPacket fields are empty.
…ectionLoop - Check closeRequest/isInterruptionRequested at loop entry - Add debug output when exiting due to interruption/close request This helps trace shutdown behavior and prevents unnecessary loop iterations.
Remove duplicate disconnect/close/deleteLater from end of run()'s client path. Cleanup now happens only once, in the worker thread, when loop exits. Eliminates race risk and makes shutdown logic clearer.
Replace unsafe clientConnection = &sock (stack variable) with new QSslSocket + setSocketDescriptor. Prevents dangling pointer when sock goes out of scope. Adds error handling if setSocketDescriptor fails.
- Set closeRequest and requestInterruption - Remove direct close/waitForDisconnected calls - Log caller thread for debugging This prevents cross-thread socket access crashes when called from main thread.
- Add detailed debug logs with connection ID - Wait briefly (2s) after signaling thread to stop - Remove redundant post-flag check (always true) - Emit disconnected() after wait attempt Improves traceability without blocking forever.
Include the new lifecycle test file (with QApplication) in CMake and run it under the GUI/event-loop runner. Ensures real socket/thread behavior is tested consistently.
Prevents re-entrant close() calls (e.g. signal loops or double clicks). Initialized to false in header.
Introduces forceShutdown() which sets closeRequest/interruption and calls abort() on the socket if connected. This safely breaks waitForReadyRead(), waitForConnected(), etc. from another thread without races. Required for reliable destructor cleanup.
Moves the full sequence (flags → wait → forceShutdown → final wait → logging) into a reusable helper method. Reduces duplication and makes behavior consistent between close() and destructor.
…-entrancy guard - Calls shutdownThreadSafely(2000) for graceful shutdown - Adds m_isClosing guard to prevent multiple concurrent close calls - Removes redundant state check after flag reset - Improves logging with 'initiated' and 'completed' messages This makes close() safer and easier to trace.
…vely - Removes call to close() (avoids duplication and long blocks in dtor) - Only runs if thread is still running (user forgot close()) - Uses shorter timeout (1000 ms) + forceShutdown fallback - Logs warning when forcing cleanup This ensures RAII: destructor is fast, safe, and non-blocking.
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.
Before submitting a pull request: