SmartPtr: Support detect memory leak by valgrind. v6.0.132 (#4102)

1. Support detect memory leak by valgrind. 2. Free the http handler entry. 3. Free the stack of ST. --- Co-authored-by: Jacob Su <suzp1984@gmail.com>
ossrs · Jun 29, 2024 · 7ab012c · 7ab012c · winlinvip · Jul 4, 2024
1 parent ea7e2c2
commit 7ab012c
Show file tree

Hide file tree

Showing 18 changed files with 286 additions and 69 deletions.
diff --git a/trunk/3rdparty/st-srs/Makefile b/trunk/3rdparty/st-srs/Makefile
@@ -191,6 +191,9 @@ endif
 #
 # make EXTRA_CFLAGS=-DDEBUG_STATS
 #
+# or cache the stack and reuse it:
+# make EXTRA_CFLAGS=-DMD_CACHE_STACK
+#
 # or enable the coverage for utest:
 # make UTEST_FLAGS="-fprofile-arcs -ftest-coverage"
 #

diff --git a/trunk/3rdparty/st-srs/stk.c b/trunk/3rdparty/st-srs/stk.c
@@ -57,13 +57,15 @@ __thread int _st_num_free_stacks = 0;
 __thread int _st_randomize_stacks = 0;
 
 static char *_st_new_stk_segment(int size);
+static void _st_delete_stk_segment(char *vaddr, int size);
 
 _st_stack_t *_st_stack_new(int stack_size)
 {
     _st_clist_t *qp;
     _st_stack_t *ts;
     int extra;
-
+
+#ifdef MD_CACHE_STACK
     for (qp = _st_free_stacks.next; qp != &_st_free_stacks; qp = qp->next) {
         ts = _ST_THREAD_STACK_PTR(qp);
         if (ts->stk_size >= stack_size) {
@@ -75,11 +77,31 @@ _st_stack_t *_st_stack_new(int stack_size)
             return ts;
         }
     }
+#endif
+
+    extra = _st_randomize_stacks ? _ST_PAGE_SIZE : 0;
+#ifndef MD_CACHE_STACK
+    for (qp = _st_free_stacks.next; qp != &_st_free_stacks;) {
+        ts = _ST_THREAD_STACK_PTR(qp);
+        // Before qp is freed, move to next one, because the qp will be freed when free the ts.
+        qp = qp->next;
+
+        ST_REMOVE_LINK(&ts->links);
+        _st_num_free_stacks--;
+
+#if defined(DEBUG) && !defined(MD_NO_PROTECT)
+        mprotect(ts->vaddr, REDZONE, PROT_READ | PROT_WRITE);
+        mprotect(ts->stk_top + extra, REDZONE, PROT_READ | PROT_WRITE);
+#endif
+
+        _st_delete_stk_segment(ts->vaddr, ts->vaddr_size);
+        free(ts);
+    }
+#endif
 
     /* Make a new thread stack object. */
     if ((ts = (_st_stack_t *)calloc(1, sizeof(_st_stack_t))) == NULL)
         return NULL;
-    extra = _st_randomize_stacks ? _ST_PAGE_SIZE : 0;
     ts->vaddr_size = stack_size + 2*REDZONE + extra;
     ts->vaddr = _st_new_stk_segment(ts->vaddr_size);
     if (!ts->vaddr) {
@@ -114,7 +136,7 @@ void _st_stack_free(_st_stack_t *ts)
 {
     if (!ts)
         return;
-    
+
     /* Put the stack on the free list */
     ST_APPEND_LINK(&ts->links, _st_free_stacks.prev);
     _st_num_free_stacks++;
@@ -152,8 +174,6 @@ static char *_st_new_stk_segment(int size)
 }
 
 
-/* Not used */
-#if 0
 void _st_delete_stk_segment(char *vaddr, int size)
 {
 #ifdef MALLOC_STACK
@@ -162,7 +182,6 @@ void _st_delete_stk_segment(char *vaddr, int size)
     (void) munmap(vaddr, size);
 #endif
 }
-#endif
 
 int st_randomize_stacks(int on)
 {

diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md
@@ -7,6 +7,7 @@ The changelog for SRS.
 <a name="v6-changes"></a>
 
 ## SRS 6.0 Changelog
+* v6.0, 2024-06-29, Merge [#4102](https://github.com/ossrs/srs/pull/4102): SmartPtr: Support detect memory leak by valgrind. v6.0.132 (#4102)
 * v6.0, 2024-06-21, Merge [#4100](https://github.com/ossrs/srs/pull/4100): Fix security scan problems. v6.0.131 (#4100)
 * v6.0, 2024-06-21, Merge [#4097](https://github.com/ossrs/srs/pull/4097): SmartPtr: Support load test for source by srs-bench. v6.0.130 (#4097)
 * v6.0, 2024-06-15, Merge [#4089](https://github.com/ossrs/srs/pull/4089): SmartPtr: Support shared ptr for live source. v6.0.129 (#4089)

diff --git a/trunk/research/st/.gitignore b/trunk/research/st/.gitignore
@@ -2,3 +2,4 @@ udp-server
 udp-client
 cost
 cost.log
+thread-join
diff --git a/trunk/research/st/thread-join.cpp b/trunk/research/st/thread-join.cpp
@@ -0,0 +1,36 @@
+/*
+g++ thread-join.cpp ../../objs/st/libst.a -g -O0 -o thread-join && ./thread-join
+*/
+#include <stdio.h>
+#include <stdlib.h>
+#include "../../objs/st/st.h"
+
+void* pfn(void* arg) {
+    printf("pid=%d, coroutine is ok\n", ::getpid());
+    return NULL;
+}
+
+int main(int argc, char** argv) {
+    st_init();
+
+    printf("pid=%d, create coroutine #1\n", ::getpid());
+    st_thread_t thread = st_thread_create(pfn, NULL, 1, 0);
+    st_thread_join(thread, NULL);
+
+    st_usleep(100 * 1000);
+
+    printf("pid=%d, create coroutine #2\n", ::getpid());
+    thread = st_thread_create(pfn, NULL, 1, 0);
+    st_thread_join(thread, NULL);
+
+    st_usleep(100 * 1000);
+
+    printf("pid=%d, create coroutine #3\n", ::getpid());
+    thread = st_thread_create(pfn, NULL, 1, 0);
+    st_thread_join(thread, NULL);
+
+    printf("done\n");
+    st_thread_exit(NULL);
+    return 0;
+}
+
diff --git a/trunk/src/app/srs_app_conn.cpp b/trunk/src/app/srs_app_conn.cpp
@@ -54,8 +54,8 @@ SrsResourceManager::~SrsResourceManager()
         trd->stop();
 
         srs_freep(trd);
-        srs_cond_destroy(cond);
     }
+    srs_cond_destroy(cond);
 
     clear();
 

diff --git a/trunk/src/app/srs_app_hourglass.cpp b/trunk/src/app/srs_app_hourglass.cpp
@@ -166,7 +166,7 @@ void SrsFastTimer::unsubscribe(ISrsFastTimer* timer)
 {
     vector<ISrsFastTimer*>::iterator it = std::find(handlers_.begin(), handlers_.end(), timer);
     if (it != handlers_.end()) {
-        it = handlers_.erase(it);
+        handlers_.erase(it);
     }
 }
 

diff --git a/trunk/src/app/srs_app_http_stream.cpp b/trunk/src/app/srs_app_http_stream.cpp
@@ -1073,9 +1073,6 @@ void SrsHttpStreamServer::http_unmount(SrsRequest* r)
     SrsBufferCache* cache = entry->cache;
     SrsAutoFree(SrsBufferCache, cache);
 
-    // Unmount the HTTP handler.
-    mux.unhandle(entry->mount, stream);
-
     // Notify cache and stream to stop.
     if (stream->entry) stream->entry->enabled = false;
     cache->stop();
@@ -1089,6 +1086,10 @@ void SrsHttpStreamServer::http_unmount(SrsRequest* r)
         srs_usleep(100 * SRS_UTIME_MILLISECONDS);
     }
 
+    // Unmount the HTTP handler, which will free the entry. Note that we must free it after cache and
+    // stream stopped for it uses it.
+    mux.unhandle(entry->mount, stream);
+
     srs_trace("http: unmount flv stream for sid=%s, i=%d", sid.c_str(), i);
 }
 

diff --git a/trunk/src/app/srs_app_hybrid.cpp b/trunk/src/app/srs_app_hybrid.cpp
@@ -25,55 +25,55 @@ extern SrsPps* _srs_pps_conn;
 extern SrsPps* _srs_pps_dispose;
 
 #if defined(SRS_DEBUG) && defined(SRS_DEBUG_STATS)
-extern unsigned long long _st_stat_recvfrom;
-extern unsigned long long _st_stat_recvfrom_eagain;
-extern unsigned long long _st_stat_sendto;
-extern unsigned long long _st_stat_sendto_eagain;
+extern __thread unsigned long long _st_stat_recvfrom;
+extern __thread unsigned long long _st_stat_recvfrom_eagain;
+extern __thread unsigned long long _st_stat_sendto;
+extern __thread unsigned long long _st_stat_sendto_eagain;
 SrsPps* _srs_pps_recvfrom = NULL;
 SrsPps* _srs_pps_recvfrom_eagain = NULL;
 SrsPps* _srs_pps_sendto = NULL;
 SrsPps* _srs_pps_sendto_eagain = NULL;
 
-extern unsigned long long _st_stat_read;
-extern unsigned long long _st_stat_read_eagain;
-extern unsigned long long _st_stat_readv;
-extern unsigned long long _st_stat_readv_eagain;
-extern unsigned long long _st_stat_writev;
-extern unsigned long long _st_stat_writev_eagain;
+extern __thread unsigned long long _st_stat_read;
+extern __thread unsigned long long _st_stat_read_eagain;
+extern __thread unsigned long long _st_stat_readv;
+extern __thread unsigned long long _st_stat_readv_eagain;
+extern __thread unsigned long long _st_stat_writev;
+extern __thread unsigned long long _st_stat_writev_eagain;
 SrsPps* _srs_pps_read = NULL;
 SrsPps* _srs_pps_read_eagain = NULL;
 SrsPps* _srs_pps_readv = NULL;
 SrsPps* _srs_pps_readv_eagain = NULL;
 SrsPps* _srs_pps_writev = NULL;
 SrsPps* _srs_pps_writev_eagain = NULL;
 
-extern unsigned long long _st_stat_recvmsg;
-extern unsigned long long _st_stat_recvmsg_eagain;
-extern unsigned long long _st_stat_sendmsg;
-extern unsigned long long _st_stat_sendmsg_eagain;
+extern __thread unsigned long long _st_stat_recvmsg;
+extern __thread unsigned long long _st_stat_recvmsg_eagain;
+extern __thread unsigned long long _st_stat_sendmsg;
+extern __thread unsigned long long _st_stat_sendmsg_eagain;
 SrsPps* _srs_pps_recvmsg = NULL;
 SrsPps* _srs_pps_recvmsg_eagain = NULL;
 SrsPps* _srs_pps_sendmsg = NULL;
 SrsPps* _srs_pps_sendmsg_eagain = NULL;
 
-extern unsigned long long _st_stat_epoll;
-extern unsigned long long _st_stat_epoll_zero;
-extern unsigned long long _st_stat_epoll_shake;
-extern unsigned long long _st_stat_epoll_spin;
+extern __thread unsigned long long _st_stat_epoll;
+extern __thread unsigned long long _st_stat_epoll_zero;
+extern __thread unsigned long long _st_stat_epoll_shake;
+extern __thread unsigned long long _st_stat_epoll_spin;
 SrsPps* _srs_pps_epoll = NULL;
 SrsPps* _srs_pps_epoll_zero = NULL;
 SrsPps* _srs_pps_epoll_shake = NULL;
 SrsPps* _srs_pps_epoll_spin = NULL;
 
-extern unsigned long long _st_stat_sched_15ms;
-extern unsigned long long _st_stat_sched_20ms;
-extern unsigned long long _st_stat_sched_25ms;
-extern unsigned long long _st_stat_sched_30ms;
-extern unsigned long long _st_stat_sched_35ms;
-extern unsigned long long _st_stat_sched_40ms;
-extern unsigned long long _st_stat_sched_80ms;
-extern unsigned long long _st_stat_sched_160ms;
-extern unsigned long long _st_stat_sched_s;
+extern __thread unsigned long long _st_stat_sched_15ms;
+extern __thread unsigned long long _st_stat_sched_20ms;
+extern __thread unsigned long long _st_stat_sched_25ms;
+extern __thread unsigned long long _st_stat_sched_30ms;
+extern __thread unsigned long long _st_stat_sched_35ms;
+extern __thread unsigned long long _st_stat_sched_40ms;
+extern __thread unsigned long long _st_stat_sched_80ms;
+extern __thread unsigned long long _st_stat_sched_160ms;
+extern __thread unsigned long long _st_stat_sched_s;
 SrsPps* _srs_pps_sched_15ms = NULL;
 SrsPps* _srs_pps_sched_20ms = NULL;
 SrsPps* _srs_pps_sched_25ms = NULL;
@@ -96,11 +96,12 @@ SrsPps* _srs_pps_clock_160ms = NULL;
 SrsPps* _srs_pps_timer_s = NULL;
 
 #if defined(SRS_DEBUG) && defined(SRS_DEBUG_STATS)
-extern int _st_active_count;
-extern unsigned long long _st_stat_thread_run;
-extern unsigned long long _st_stat_thread_idle;
-extern unsigned long long _st_stat_thread_yield;
-extern unsigned long long _st_stat_thread_yield2;
+extern __thread int _st_active_count;
+extern __thread int _st_num_free_stacks;
+extern __thread unsigned long long _st_stat_thread_run;
+extern __thread unsigned long long _st_stat_thread_idle;
+extern __thread unsigned long long _st_stat_thread_yield;
+extern __thread unsigned long long _st_stat_thread_yield2;
 SrsPps* _srs_pps_thread_run = NULL;
 SrsPps* _srs_pps_thread_idle = NULL;
 SrsPps* _srs_pps_thread_yield = NULL;
@@ -135,19 +136,20 @@ SrsHybridServer::SrsHybridServer()
 
 SrsHybridServer::~SrsHybridServer()
 {
-    srs_freep(clock_monitor_);
-
-    srs_freep(timer20ms_);
-    srs_freep(timer100ms_);
-    srs_freep(timer1s_);
-    srs_freep(timer5s_);
-
+    // We must free servers first, because it may depend on the timers of hybrid server.
     vector<ISrsHybridServer*>::iterator it;
     for (it = servers.begin(); it != servers.end(); ++it) {
         ISrsHybridServer* server = *it;
         srs_freep(server);
     }
     servers.clear();
+
+    srs_freep(clock_monitor_);
+
+    srs_freep(timer20ms_);
+    srs_freep(timer100ms_);
+    srs_freep(timer1s_);
+    srs_freep(timer5s_);
 }
 
 void SrsHybridServer::register_server(ISrsHybridServer* svr)
@@ -237,8 +239,6 @@ void SrsHybridServer::stop()
         ISrsHybridServer* server = *it;
         server->stop();
     }
-
-    srs_st_destroy();
 }
 
 SrsServerAdapter* SrsHybridServer::srs()
@@ -372,8 +372,8 @@ srs_error_t SrsHybridServer::on_timer(srs_utime_t interval)
 #if defined(SRS_DEBUG) && defined(SRS_DEBUG_STATS)
     _srs_pps_thread_run->update(_st_stat_thread_run); _srs_pps_thread_idle->update(_st_stat_thread_idle);
     _srs_pps_thread_yield->update(_st_stat_thread_yield); _srs_pps_thread_yield2->update(_st_stat_thread_yield2);
-    if (_st_active_count > 0 || _srs_pps_thread_run->r10s() || _srs_pps_thread_idle->r10s() || _srs_pps_thread_yield->r10s() || _srs_pps_thread_yield2->r10s()) {
-        snprintf(buf, sizeof(buf), ", co=%d,%d,%d, yield=%d,%d", _st_active_count, _srs_pps_thread_run->r10s(), _srs_pps_thread_idle->r10s(), _srs_pps_thread_yield->r10s(), _srs_pps_thread_yield2->r10s());
+    if (_st_active_count > 0 || _st_num_free_stacks > 0 || _srs_pps_thread_run->r10s() || _srs_pps_thread_idle->r10s() || _srs_pps_thread_yield->r10s() || _srs_pps_thread_yield2->r10s()) {
+        snprintf(buf, sizeof(buf), ", co=%d,%d,%d, stk=%d, yield=%d,%d", _st_active_count, _srs_pps_thread_run->r10s(), _srs_pps_thread_idle->r10s(), _st_num_free_stacks, _srs_pps_thread_yield->r10s(), _srs_pps_thread_yield2->r10s());
         thread_desc = buf;
     }
 #endif

diff --git a/trunk/src/app/srs_app_ingest.cpp b/trunk/src/app/srs_app_ingest.cpp
@@ -94,7 +94,7 @@ SrsIngester::SrsIngester()
     disposed = false;
 
     trd = new SrsDummyCoroutine();
-    pprint = SrsPithyPrint::create_ingester();
+    pprint_ = SrsPithyPrint::create_ingester();
 }
 
 SrsIngester::~SrsIngester()
@@ -103,6 +103,7 @@ SrsIngester::~SrsIngester()
 
     srs_freep(trd);
     clear_engines();
+    srs_freep(pprint_);
 }
 
 void SrsIngester::dispose()
@@ -466,7 +467,7 @@ srs_error_t SrsIngester::initialize_ffmpeg(SrsFFMPEG* ffmpeg, SrsConfDirective*
 
 void SrsIngester::show_ingest_log_message()
 {
-    pprint->elapse();
+    pprint_->elapse();
 
     if ((int)ingesters.size() <= 0) {
         return;
@@ -477,9 +478,9 @@ void SrsIngester::show_ingest_log_message()
     SrsIngesterFFMPEG* ingester = ingesters.at(index);
 
     // reportable
-    if (pprint->can_print()) {
+    if (pprint_->can_print()) {
         srs_trace("-> " SRS_CONSTS_LOG_INGESTER " time=%dms, ingesters=%d, #%d(alive=%dms, %s)",
-                  srsu2msi(pprint->age()), (int)ingesters.size(), index, srsu2msi(ingester->alive()), ingester->uri().c_str());
+                  srsu2msi(pprint_->age()), (int)ingesters.size(), index, srsu2msi(ingester->alive()), ingester->uri().c_str());
     }
 }
 

diff --git a/trunk/src/app/srs_app_ingest.hpp b/trunk/src/app/srs_app_ingest.hpp
@@ -55,7 +55,7 @@ class SrsIngester : public ISrsCoroutineHandler, public ISrsReloadHandler
     std::vector<SrsIngesterFFMPEG*> ingesters;
 private:
     SrsCoroutine* trd;
-    SrsPithyPrint* pprint;
+    SrsPithyPrint* pprint_;
     // Whether the ingesters are expired, for example, the listen port changed,
     // all ingesters must be restart.
     bool expired;