From ab37977fbd38e6d0888cb65999d28eb8f3b76490 Mon Sep 17 00:00:00 2001
From: johnchia <john@johnchia.com>
Date: Mon, 27 Jul 2026 23:56:51 -0700
Subject: [PATCH] rtsp: don't free the server handle while encoder threads
 still use it

main.c calls rtsp_finish() at shutdown and then keeps going for another
dozen lines before sdk_stop() finally stops the encoder threads. It never
clears the rtspHandle global, and media.c passes that global straight into
rtp_send_h26x() and rtp_send_mp3(), so every frame produced in that window
dereferences a handle that rtsp_finish() has already freed.

The guard that was supposed to catch this could not: it read the quit flag
through h->pool->sharedp->gbl, but rtsp_finish() raises that flag and then
calls threadpool_delete(h->pool), so the check consulted freed memory to
decide whether the memory was freed. Once the allocation is reused -- and
region_stop() and night_disable() run in between, so it is -- the check
reads whatever now occupies that word, passes, and the sender goes on to
lock a destroyed mutex and walk a destroyed connection list.

Keep the shutdown flag in the handle itself, and stop freeing the handle
and destroying its mutex in rtsp_finish(), so a late sender always finds a
valid mutex and a flag telling it to turn around. The connections, pools
and threadpool are still released as before. rtsp_create() runs once per
process, so what remains is one small allocation reclaimed at exit;
rtsp_create()'s own error path frees the handle explicitly, since nothing
can reach it there yet.

Seen on an Infinity6E (ssc30kq) as a SIGSEGV on every SIGTERM restart with
two clients connected, faulting a few dozen bytes into the sender's
connection-list walk on a junk pointer.
---
 src/rtsp/rtp.c  | 15 +++++++++++++--
 src/rtsp/rtsp.c | 25 ++++++++++++++++++++++---
 src/rtsp/rtsp.h |  4 ++++
 3 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/src/rtsp/rtp.c b/src/rtsp/rtp.c
index 52750dfd..f08c44e1 100644
--- a/src/rtsp/rtp.c
+++ b/src/rtsp/rtp.c
@@ -415,12 +415,19 @@ int rtp_send_h26x(rtsp_handle h, hal_vidstream *stream, char isH265)
     /* checkout RTP packet */
     DASSERT(h, return FAILURE);
 
-    if (gbl_get_quit(h->pool->sharedp->gbl)) {
+    /* Do not reach through h->pool to find out whether the server is gone:
+     * rtsp_finish() raises that quit flag and then deletes the threadpool it
+     * lives in, so the check itself read freed memory. h->finished lives in the
+     * handle, which rtsp_finish() now keeps alive for exactly this reason. */
+    rtsp_lock(h);
+    if (h->finished) {
+        rtsp_unlock(h);
 #ifdef DEBUG_RTSP
         ERR("server threads have gone already. call rtsp_finish()\n");
 #endif
         return FAILURE;
     }
+    rtsp_unlock(h);
 
     h->isH265 = isH265;
 
@@ -471,12 +478,16 @@ int rtp_send_mp3(rtsp_handle h, unsigned char *buf, size_t len)
     /* checkout RTP packet */
     DASSERT(h, return FAILURE);
 
-    if (gbl_get_quit(h->pool->sharedp->gbl)) {
+    /* See the note in rtp_send_h26x(). */
+    rtsp_lock(h);
+    if (h->finished) {
+        rtsp_unlock(h);
 #ifdef DEBUG_RTSP
         ERR("server threads have gone already. call rtsp_finish()\n");
 #endif
         return FAILURE;
     }
+    rtsp_unlock(h);
 
     h->audioPt = 14;
 
diff --git a/src/rtsp/rtsp.c b/src/rtsp/rtsp.c
index 7219c862..2e5700f7 100644
--- a/src/rtsp/rtsp.c
+++ b/src/rtsp/rtsp.c
@@ -838,6 +838,13 @@ void rtsp_finish(rtsp_handle h)
 {
     /* close every connections in the handle */
     if (h) {
+        /* Tell the encoder threads to stop reaching into this handle before any
+         * of it goes away. They call rtp_send_h26x()/rtp_send_mp3() from
+         * media.c, which is still running at this point. */
+        pthread_mutex_lock(&h->mutex);
+        h->finished = 1;
+        pthread_mutex_unlock(&h->mutex);
+
         list_destroy(&h->con_list);
 
         if (h->pool) {
@@ -854,11 +861,17 @@ void rtsp_finish(rtsp_handle h)
             mime_encoded_delete(h->sprop_pps_b64);
 
             threadpool_delete(h->pool);
+            h->pool = NULL;
         }
 
-        pthread_mutex_destroy(&h->mutex);
-
-        FREE(h);
+        /* Deliberately no pthread_mutex_destroy()/FREE(h) here. main.c calls
+         * rtsp_finish() well before sdk_stop() and never clears the rtspHandle
+         * global, so the encoder threads keep calling rtp_send_h26x() and
+         * rtp_send_mp3() with this pointer until the process exits; they need
+         * the mutex and the finished flag to stay valid in order to bail out.
+         * rtsp_create() runs once per process, so this costs a single small
+         * allocation that process exit reclaims. rtsp_create()'s own error path
+         * still frees the handle, where nothing can reach it yet. */
     }
 
     return;
@@ -897,7 +910,13 @@ rtsp_handle rtsp_create(unsigned char max_con, unsigned int port, int priority)
     return nh;
 
 error:
+    /* Nothing has published this handle yet, so unlike the shutdown path there
+     * is no encoder thread that could still dereference it. */
     rtsp_finish(nh);
+    if (nh) {
+        pthread_mutex_destroy(&nh->mutex);
+        FREE(nh);
+    }
     return NULL;
 }
 
diff --git a/src/rtsp/rtsp.h b/src/rtsp/rtsp.h
index 18c7333a..49b0d6ba 100644
--- a/src/rtsp/rtsp.h
+++ b/src/rtsp/rtsp.h
@@ -124,6 +124,10 @@ struct transfer_item_t {
 
 struct __rtsp_obj_t {
     pthread_mutex_t mutex;
+    /* Raised by rtsp_finish(). The encoder threads keep calling rtp_send_*()
+     * with this handle after that point, so it has to stay readable for the
+     * rest of the process lifetime. Guarded by mutex. */
+    char finished;
     struct list_head_t con_list;
     threadpool_handle pool;
     bufpool_handle con_pool;