DEC Setup() swaps the PrimProc connection list under running queries; SIGHUP handler calls Setup() in signal context

Two related problems in how ExeMgr, DMLProc and DDLProc rebuild their PrimProc connections (DistributedEngineComm, "DEC"). Both are from code reading at tag 23.02.24. We haven't seen either crash or hang in the field, and we haven't reproduced them. The same code is on the stable-23.10 and develop branches.

1. Setup() replaces the connection list while queries are using it

DEC::Setup() builds a new set of connections and write locks, then swaps them in (distributedenginecomm.cpp:357-363):

fWlock.swap(newLocks);

fPmConnections.swap(newClients);

pmCount.store(newPmCount, ...);

newLocks.clear();     // destroys the old write mutexes

newClients.clear();   // drops the old connections

The only lock held is fSetupMutex, which just keeps two Setup() calls apart. The code that sends to PrimProc reads both vectors with no lock at all. In writeToClient() (L1054-L1087):

if (fPmConnections[connectionId]->atTheSameHost() && fIsExeMgr) ...

ClientList::value_type client = fPmConnections[connectionId];

...

std::lock_guard lk(*(fWlock[connectionId]));

client->write(bs, NULL, senderStats);

If a Setup() runs while another thread is in that code, the sender can:

• copy a shared_ptr out of a vector whose buffer is being swapped and freed,
• lock (or still hold) a write mutex that newLocks.clear() has just destroyed,
• index past the end if the new list is shorter (the assert is compiled out in release builds).

That's undefined behaviour: possible outcomes are a crash, a hang on a destroyed mutex, or two threads writing to one socket at once.

When does Setup() run while queries are live? On 23.02.24, more often than it looks:

• ExeMgr: when a PrimProc connection drops, the listener thread sleeps 3 s and calls Setup() (L426-L431), while other queries keep sending.
• DMLProc/DDLProc: when a write fails, writeToClient() calls Setup() up to 10 times (L1109).
• DMLProc/DDLProc: on SIGHUP (problem 2). Every process: at start.

So any PrimProc restart under concurrent load can open this window.

2. The SIGHUP handler calls Setup() inside the signal handler

DDLProc and DMLProc install added_a_pm() as the SIGHUP handler (ddlproc.cpp:143-153, dmlproc.cpp:160-172). It logs, then calls Dec->Setup(); DMLProc also calls PackageHandler::clearTableAccess(). All of that takes mutexes, allocates and starts threads, none of which is async-signal-safe. If the signal arrives while the interrupted thread holds one of those locks (or the malloc lock), the handler waits on itself and the process deadlocks. It also runs the problem 1 swap from an arbitrary point in whatever thread received the signal.

Suggested fix

• Connection list: add a std::shared_mutex around the connection vectors. Senders take it shared, copy the slot's shared_ptr}}s (connection and write lock), release it, then write; never hold it across socket I/O. {{Setup() takes it exclusive only around the swap. Keep the current lock order (fSetupMutex -> eventListenerLock -> fMlock -> per-connection write lock); a sender must never wait on fSetupMutex while holding the new lock. (Not std::atomic<std::shared_ptr>: the libstdc++ in the 23.02 toolchain, gcc-toolset-11, doesn't have it.)
• SIGHUP: the handler only sets an atomic "config changed" flag. The next query, or a small helper thread, does the full Setup() outside signal context.
• Optional, only if cheap: Setup() never closes the connections it replaces, so their listener threads keep them open, and fPmReader grows by one thread per (re)connect.

Relation to MCOL-6613

Our proven fix branch for MCOL-6613 (mcol-6613) narrows problem 1 but doesn't close it. After a PrimProc restart it reconnects the dead connection in place and no longer calls Setup() from the write path, so the list isn't swapped. A full Setup() still runs at process start, on SIGHUP, and when the list was incomplete from the start. Both problems are listed under "Not done" on MCOL-6613. A fix for this ticket should be built on that branch, since it uses the same per-connection state and reconnectIfDead() hook.

How to test

Send SIGHUP to DMLProc and DDLProc repeatedly (5+ times) while concurrent DML and SELECT load is running, and separately restart PrimProc on one node under the same load. Pass: no crash, no hang, and no query errors except ones that legitimately need a PM that was removed.