Fix shutdown race conditions causing bad_weak_ptr crash (v5.4.1)
Build All Platforms / test-linux-unit (push) Waiting to run
Build All Platforms / build-windows-qt (push) Waiting to run
Build All Platforms / build-windows-daemon (push) Waiting to run
Build All Platforms / build-linux-qt (push) Waiting to run
Build All Platforms / build-linux-daemon (push) Waiting to run
Build All Platforms / build-macos (push) Waiting to run
Build All Platforms / release (push) Blocked by required conditions
Build All Platforms / test-linux-unit (push) Waiting to run
Build All Platforms / build-windows-qt (push) Waiting to run
Build All Platforms / build-windows-daemon (push) Waiting to run
Build All Platforms / build-linux-qt (push) Waiting to run
Build All Platforms / build-linux-daemon (push) Waiting to run
Build All Platforms / build-macos (push) Waiting to run
Build All Platforms / release (push) Blocked by required conditions
Fixes multiple concurrency bugs exposed during shutdown when Tor proxy connections are failing: - Reorder shutdown: stop network threads before destroying Tor V3 services - Make RPC listener responsive to fShutdown (poll_one+sleep vs blocking run_one) - Wrap StopRequests() in try/catch and drain io_service on exit - Fix leaked CNode AddRef in ThreadSocketHandler2 and ThreadMessageHandler2 (return→break so Release loop executes) - Guard vNodes.size() read with cs_vNodes lock (data race) - Guard Qt UI signal callbacks with fShutdown check (use-after-free) - Add cs_vNodes lock in CNetCleanup global destructor - Force-disconnect remaining nodes in StopNode() after threads stop - Make Tor maintenance thread sleep in 500ms intervals for prompt shutdown Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -8,7 +8,7 @@
|
||||
// These need to be macros, as version.cpp's and triangles-qt.rc's voodoo requires it
|
||||
#define CLIENT_VERSION_MAJOR 5
|
||||
#define CLIENT_VERSION_MINOR 4
|
||||
#define CLIENT_VERSION_REVISION 0
|
||||
#define CLIENT_VERSION_REVISION 1
|
||||
#define CLIENT_VERSION_BUILD 0
|
||||
|
||||
// Converts the parameter X to a string after macro replacement on X has been performed.
|
||||
|
||||
+6
-2
@@ -176,6 +176,12 @@ void Shutdown(void* parg)
|
||||
}
|
||||
|
||||
SecureMsgShutdown();
|
||||
|
||||
// Stop network threads FIRST so nothing references Tor objects
|
||||
nTransactionsUpdated++;
|
||||
StopNode();
|
||||
|
||||
// NOW safe to destroy Tor state - all threads have stopped
|
||||
ShutdownTorV3();
|
||||
StopEmbeddedTor();
|
||||
|
||||
@@ -194,10 +200,8 @@ void Shutdown(void* parg)
|
||||
pNotificationQueue = NULL;
|
||||
}
|
||||
|
||||
nTransactionsUpdated++;
|
||||
// CTxDB().Close();
|
||||
bitdb.Flush(false);
|
||||
StopNode();
|
||||
bitdb.Flush(true);
|
||||
fs::remove(GetPidFile());
|
||||
UnregisterWallet(pwalletMain);
|
||||
|
||||
+33
-10
@@ -878,10 +878,19 @@ void ThreadSocketHandler2(void* parg)
|
||||
}
|
||||
}
|
||||
}
|
||||
if (vNodes.size() != nPrevNodeCount)
|
||||
{
|
||||
nPrevNodeCount = vNodes.size();
|
||||
uiInterface.NotifyNumConnectionsChanged(vNodes.size());
|
||||
// Read vNodes.size() under the lock to avoid data race
|
||||
unsigned int nNodeCount;
|
||||
{
|
||||
LOCK(cs_vNodes);
|
||||
nNodeCount = vNodes.size();
|
||||
}
|
||||
if (nNodeCount != nPrevNodeCount)
|
||||
{
|
||||
nPrevNodeCount = nNodeCount;
|
||||
if (!fShutdown)
|
||||
uiInterface.NotifyNumConnectionsChanged(nNodeCount);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1013,7 +1022,7 @@ void ThreadSocketHandler2(void* parg)
|
||||
for (CNode* pnode : vNodesCopy)
|
||||
{
|
||||
if (fShutdown)
|
||||
return;
|
||||
break;
|
||||
|
||||
//
|
||||
// Receive
|
||||
@@ -1039,7 +1048,7 @@ void ThreadSocketHandler2(void* parg)
|
||||
if (!pnode->ReceiveMsgBytes(pchBuf, nBytes))
|
||||
pnode->CloseSocketDisconnect();
|
||||
pnode->nLastRecv = GetTime();
|
||||
pnode->nRecvBytes += nBytes;
|
||||
pnode->nRecvBytes += nBytes;
|
||||
}
|
||||
else if (nBytes == 0)
|
||||
{
|
||||
@@ -1104,6 +1113,8 @@ void ThreadSocketHandler2(void* parg)
|
||||
pnode->Release();
|
||||
}
|
||||
|
||||
if (fShutdown)
|
||||
return;
|
||||
MilliSleep(10);
|
||||
}
|
||||
}
|
||||
@@ -1832,6 +1843,9 @@ void ThreadMessageHandler2(void* parg)
|
||||
pnodeTrickle = vNodesCopy[GetRand(vNodesCopy.size())];
|
||||
for (CNode* pnode : vNodesCopy)
|
||||
{
|
||||
if (fShutdown)
|
||||
break;
|
||||
|
||||
// Receive messages
|
||||
{
|
||||
TRY_LOCK(pnode->cs_vRecvMsg, lockRecv);
|
||||
@@ -1839,8 +1853,6 @@ void ThreadMessageHandler2(void* parg)
|
||||
if (!ProcessMessages(pnode))
|
||||
pnode->CloseSocketDisconnect();
|
||||
}
|
||||
if (fShutdown)
|
||||
return;
|
||||
|
||||
// Send messages
|
||||
{
|
||||
@@ -1848,8 +1860,6 @@ void ThreadMessageHandler2(void* parg)
|
||||
if (lockSend)
|
||||
SendMessages(pnode, pnode == pnodeTrickle);
|
||||
}
|
||||
if (fShutdown)
|
||||
return;
|
||||
}
|
||||
|
||||
{
|
||||
@@ -2180,6 +2190,18 @@ bool StopNode()
|
||||
}
|
||||
MilliSleep(50);
|
||||
DumpAddresses();
|
||||
|
||||
// Force-disconnect and clean up all remaining nodes now that threads have stopped.
|
||||
// Close sockets first so any lingering I/O fails immediately.
|
||||
{
|
||||
LOCK(cs_vNodes);
|
||||
for (CNode* pnode : vNodes)
|
||||
{
|
||||
pnode->CloseSocketDisconnect();
|
||||
pnode->Cleanup();
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -2191,7 +2213,8 @@ public:
|
||||
}
|
||||
~CNetCleanup()
|
||||
{
|
||||
// Close sockets
|
||||
// Close sockets - acquire lock in case other threads are still winding down
|
||||
LOCK(cs_vNodes);
|
||||
for (CNode* pnode : vNodes)
|
||||
if (pnode->hSocket != INVALID_SOCKET)
|
||||
closesocket(pnode->hSocket);
|
||||
|
||||
@@ -193,13 +193,16 @@ static void NotifyBlocksChanged(ClientModel *clientmodel)
|
||||
|
||||
static void NotifyNumConnectionsChanged(ClientModel *clientmodel, int newNumConnections)
|
||||
{
|
||||
// Too noisy: OutputDebugStringF("NotifyNumConnectionsChanged %i\n", newNumConnections);
|
||||
// Don't queue UI updates during shutdown - ClientModel may be destroyed
|
||||
// before Qt processes the queued invocation, causing use-after-free.
|
||||
if (fShutdown) return;
|
||||
QMetaObject::invokeMethod(clientmodel, "updateNumConnections", Qt::QueuedConnection,
|
||||
Q_ARG(int, newNumConnections));
|
||||
}
|
||||
|
||||
static void NotifyAlertChanged(ClientModel *clientmodel, const uint256 &hash, ChangeType status)
|
||||
{
|
||||
if (fShutdown) return;
|
||||
OutputDebugStringF("NotifyAlertChanged %s status=%i\n", hash.GetHex().c_str(), status);
|
||||
QMetaObject::invokeMethod(clientmodel, "updateAlert", Qt::QueuedConnection,
|
||||
Q_ARG(QString, QString::fromStdString(hash.GetHex())),
|
||||
|
||||
@@ -2206,7 +2206,9 @@ void ThreadTorMaintenance(void* parg)
|
||||
|
||||
while (!fShutdown)
|
||||
{
|
||||
MilliSleep(30000); // check every 30 seconds
|
||||
// Sleep in short intervals so the thread exits promptly on shutdown
|
||||
for (int i = 0; i < 60 && !fShutdown; i++)
|
||||
MilliSleep(500);
|
||||
if (fShutdown) break;
|
||||
|
||||
// --- Tor health check & auto-restart ---
|
||||
@@ -2243,7 +2245,8 @@ void ThreadTorMaintenance(void* parg)
|
||||
else
|
||||
{
|
||||
printf("WARNING: Tor restart failed, retrying in %d seconds\n", restartBackoffSec);
|
||||
MilliSleep(restartBackoffSec * 1000);
|
||||
for (int i = 0; i < restartBackoffSec * 2 && !fShutdown; i++)
|
||||
MilliSleep(500);
|
||||
if (restartBackoffSec < 300)
|
||||
restartBackoffSec *= 2;
|
||||
}
|
||||
|
||||
+17
-2
@@ -946,9 +946,24 @@ void ThreadRPCServer2(void* parg)
|
||||
|
||||
vnThreadsRunning[THREAD_RPCLISTENER]--;
|
||||
while (!fShutdown)
|
||||
io_service.run_one();
|
||||
{
|
||||
// Use poll_one + sleep instead of blocking run_one so the thread
|
||||
// remains responsive to fShutdown and can exit promptly.
|
||||
if (!io_service.poll_one())
|
||||
{
|
||||
io_service.restart();
|
||||
MilliSleep(50);
|
||||
}
|
||||
}
|
||||
vnThreadsRunning[THREAD_RPCLISTENER]++;
|
||||
StopRequests();
|
||||
|
||||
// Safely shut down: close acceptors, then drain any remaining handlers
|
||||
try {
|
||||
StopRequests();
|
||||
} catch (...) {
|
||||
// Absorb bad_weak_ptr or other exceptions from stale tracked slots
|
||||
}
|
||||
io_service.poll(); // process cancellation callbacks so shared_ptrs are released
|
||||
}
|
||||
|
||||
class JSONRequest
|
||||
|
||||
Reference in New Issue
Block a user