No Child Left Behind

src/modules/DMShell.cpp

@@ -31,8 +31,6 @@ constexpr uint16_t PTY_ROWS_DEFAULT = 40;
 constexpr size_t MAX_MESSAGE_SIZE = 200;
 constexpr size_t REPLAY_REQUEST_SIZE = sizeof(uint32_t);
 constexpr size_t HEARTBEAT_STATUS_SIZE = sizeof(uint32_t) * 2;
-constexpr uint32_t CHILD_EXIT_WAIT_TIMEOUT_MS = 1500;
-constexpr uint32_t CHILD_EXIT_POLL_INTERVAL_MS = 50;
 
 void encodeUint32BE(uint8_t *dest, uint32_t value)
 {
@@ -177,6 +175,8 @@ ProcessMessage DMShellModule::handleReceived(const meshtastic_MeshPacket &mp)
 
 int32_t DMShellModule::runOnce()
 {
+    processPendingChildReap();
+
     if (!session.active) {
         return 100;
     }
@@ -364,9 +364,14 @@ void DMShellModule::closeSession(const char *reason, bool notifyPeer)
     }
 
     if (session.childPid > 0) {
-        kill(session.childPid, SIGTERM);
-        int status = 0;
-        waitpid(session.childPid, &status, WNOHANG);
+        // Run this to avoid forgetting a child
+        processPendingChildReap();
+
+        if (kill(session.childPid, SIGTERM) < 0 && errno != ESRCH) {
+            LOG_WARN("DMShell: failed to send SIGTERM to pid=%d errno=%d", session.childPid, errno);
+        }
+
+        pendingChildPid = session.childPid;
         session.childPid = -1;
     }
 
@@ -387,6 +392,34 @@ void DMShellModule::reapChildIfExited()
     }
 }
 
+void DMShellModule::processPendingChildReap()
+{
+    if (pendingChildPid <= 0) {
+        return;
+    }
+
+    int status = 0;
+    const pid_t result = waitpid(pendingChildPid, &status, WNOHANG);
+
+    if (result == pendingChildPid || (result < 0 && errno == ECHILD)) {
+        pendingChildPid = -1;
+        return;
+    }
+
+    if (result < 0) {
+        LOG_WARN("DMShell: waitpid failed for pid=%d errno=%d", pendingChildPid, errno);
+        pendingChildPid = -1;
+        return;
+    }
+
+    if (pendingChildPid > 0) {
+        if (kill(pendingChildPid, SIGKILL) < 0 && errno != ESRCH) {
+            LOG_WARN("DMShell: failed to send SIGKILL to pid=%d errno=%d", pendingChildPid, errno);
+        }
+        pendingChildPid = -1;
+    }
+}
+
 void DMShellModule::rememberSentFrame(meshtastic_RemoteShell_OpCode op, uint32_t sessionId, uint32_t seq, const uint8_t *payload,
                                       size_t payloadLen, uint32_t cols, uint32_t rows, uint32_t ackSeq, uint32_t flags)
 {

src/modules/DMShell.h

@@ -66,6 +66,7 @@ class DMShellModule : private concurrency::OSThread, public SinglePortModule
     static constexpr uint32_t SESSION_IDLE_TIMEOUT_MS = 5 * 60 * 1000;
 
     DMShellSession session;
+    pid_t pendingChildPid = -1;
 
     bool parseFrame(const meshtastic_MeshPacket &mp, DMShellFrame &outFrame);
     bool isAuthorizedPacket(const meshtastic_MeshPacket &mp) const;
@@ -75,6 +76,7 @@ class DMShellModule : private concurrency::OSThread, public SinglePortModule
     bool writeSessionInput(const DMShellFrame &frame);
     void closeSession(const char *reason, bool notifyPeer);
     void reapChildIfExited();
+    void processPendingChildReap();
 
     void rememberSentFrame(meshtastic_RemoteShell_OpCode op, uint32_t sessionId, uint32_t seq, const uint8_t *payload,
                            size_t payloadLen, uint32_t cols, uint32_t rows, uint32_t ackSeq, uint32_t flags);