SmartPtr: Support detect memory leak by valgrind. v6.0.132 by winlinvip · Pull Request #4102 · ossrs/srs

winlinvip · 2024-06-21T10:05:55Z

Support detect memory leak by valgrind.
Free the http handler entry.
Free the stack of ST.

suzp1984 · 2024-06-27T13:30:18Z

trunk/3rdparty/st-srs/Makefile

 # make EXTRA_CFLAGS=-DDEBUG_STATS
 #
+# or cache the stack and reuse it:
+# make EXTRA_CFLAGS=-DMD_CACHE_STACK


why not add configure args to bypass MD_CACHE_STACK to st-srs, like MD_VALGRIND?

I want to not cache the stack by default, while this macro is used to enable the stack cache.

Won't fix.

suzp1984 · 2024-06-27T14:31:45Z

Here is my env to testing & verify valgrind memory leak, there are a lot of outputs need to analysis, so I align my test env here.

Because I didn't find an image with valgrind in https://hub.docker.com/r/ossrs/srs/tags, I make my own one based on ossrs/srs:ubuntu20: start this image, running apt update -y && apt install -y valgrind, tag this container to an image, then I have the docker image to do the testing.

I would suggest tag an image with all the testing tools to the srs public docker hub.

Then build srs: (ossrs/srs:valgrind is the docker image in my local env)

docker run -it --rm -v pwd:/srs -w /srs ossrs/srs:valgrind bash -c "make clean_st"
docker run -it --rm -v pwd:/srs -w /srs ossrs/srs:valgrind bash -c "./configure --valgrind=on && make"

and run the srs with valgrind:

docker run -p 1935:1935 -p 1985:1985 -p 8080:8080 -p 8085:8085 --env CANDIDATE=$(ifconfig en0 inet| grep 'inet '|awk '{print $2}') -p 8000:8000/udp -it --rm -v pwd:/srs -w /srs ossrs/srs:valgrind valgrind --leak-check=full ./objs/srs -c conf/rtmp2rtc.conf

do live streams publish and play, close the srs by ctrl + \ or ctrl + c, check the output.

suzp1984 · 2024-06-27T14:47:21Z

trunk/3rdparty/st-srs/stk.c

            ts->links.next = NULL;
            ts->links.prev = NULL;
-            return ts;
+            returnt ts;


is it a mistake? returnt -> return.

It's a mistake.
To reproduce, I need to add -DMD_CACHE_STACK to auto/depends.sh, run make clean_st, then ./configure && make.

srs/trunk/auto/depends.sh

Lines 283 to 297 in ea7e2c2

# Patched ST from https://github.com/ossrs/state-threads/tree/srs

if [[ -f ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st/libst.a ]]; then

rm -rf ${SRS_OBJS}/st && cp -rf ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st ${SRS_OBJS}/ &&

echo "The state-threads is ok."

else

echo "Building state-threads." &&

rm -rf ${SRS_OBJS}/${SRS_PLATFORM}/st-srs ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st ${SRS_OBJS}/st &&

cp -rf ${SRS_WORKDIR}/3rdparty/st-srs ${SRS_OBJS}/${SRS_PLATFORM}/ &&

env EXTRA_CFLAGS="${_ST_EXTRA_CFLAGS}" make -C ${SRS_OBJS}/${SRS_PLATFORM}/st-srs ${_ST_MAKE_ARGS} &&

mkdir -p ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st &&

cp -f ${SRS_OBJS}/${SRS_PLATFORM}/st-srs/${_ST_OBJ}/st.h ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st/ &&

cp -f ${SRS_OBJS}/${SRS_PLATFORM}/st-srs/${_ST_OBJ}/libst.a ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st/ &&

cp -rf ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st ${SRS_OBJS}/ &&

echo "The state-threads is ok."

fi

st-srs never spend too much time to build, maybe need to consider whether it's a good idea to build st-srs in this inconvenient way.

Fixed.

BTW: You can use configure to enable the macro:

./configure -h |grep flags # --extra-flags=<EFLAGS> Set EFLAGS as CFLAGS and CXXFLAGS. Also passed to ST as EXTRA_CFLAGS.

suzp1984 · 2024-06-27T15:53:45Z

Here is my env to testing & verify valgrind memory leak, there are a lot of outputs need to analysis, so I align my test env here.

Because I didn't find an image with valgrind in https://hub.docker.com/r/ossrs/srs/tags, I make my own one based on ossrs/srs:ubuntu20: start this image, running apt update -y && apt install -y valgrind, tag this container to an image, then I have the docker image to do the testing.

I would suggest tag an image with all the testing tools to the srs public docker hub.

Then build srs: (ossrs/srs:valgrind is the docker image in my local env)

docker run -it --rm -v pwd:/srs -w /srs ossrs/srs:valgrind bash -c "make clean_st"
docker run -it --rm -v pwd:/srs -w /srs ossrs/srs:valgrind bash -c "./configure --valgrind=on && make"

and run the srs with valgrind:

docker run -p 1935:1935 -p 1985:1985 -p 8080:8080 -p 8085:8085 --env CANDIDATE=$(ifconfig en0 inet| grep 'inet '|awk '{print $2}') -p 8000:8000/udp -it --rm -v pwd:/srs -w /srs ossrs/srs:valgrind valgrind --leak-check=full ./objs/srs -c conf/rtmp2rtc.conf

do live streams publish and play, close the srs by ctrl + \ or ctrl + c, check the output.

By this way, I found another definitely memory leak:

==1== 48 bytes in 1 blocks are definitely lost in loss record 48 of 145
==1==    at 0x484A3C4: operator new(unsigned long) (in /usr/lib/aarch64-linux-gnu/valgrind/vgpreload_memcheck-arm64-linux.so)
==1==    by 0x5BB90B: SrsPithyPrint::create_ingester() (srs_app_pithy_print.cpp:261)
==1==    by 0x5D842B: SrsIngester::SrsIngester() (srs_app_ingest.cpp:97)
==1==    by 0x517BE3: SrsServer::SrsServer() (srs_app_server.cpp:361)
==1==    by 0x51E4DB: SrsServerAdapter::SrsServerAdapter() (srs_app_server.cpp:1343)
==1==    by 0x6B842F: run_hybrid_server(void*) (srs_main_server.cpp:494)
==1==    by 0x6333F7: SrsThreadPool::start(void*) (srs_app_threads.cpp:939)
==1==    by 0x4881623: start_thread (pthread_create.c:477)
==1==    by 0x4C3049B: thread_start (clone.S:78)

srs/trunk/src/app/srs_app_ingest.cpp

Lines 89 to 106 in ea7e2c2

    
           SrsIngester::SrsIngester() 
        
           { 
        
               _srs_config->subscribe(this); 
        
               expired = false; 
        
               disposed = false; 
        
               trd = new SrsDummyCoroutine(); 
        
               pprint = SrsPithyPrint::create_ingester(); 
        
           } 
        
           SrsIngester::~SrsIngester() 
        
           { 
        
               _srs_config->unsubscribe(this); 
        
               srs_freep(trd); 
        
               clear_engines(); 
        
           }

srs_freep(pprint), maybe rename pprint to pprint_, if there is a private member naming convention in this way.

suzp1984 · 2024-06-27T16:00:46Z

trunk/3rdparty/st-srs/stk.c

+        _st_delete_stk_segment(ts->vaddr, ts->vaddr_size);
+        free(ts);
+    }
+#endif


only rarely a few part of codes in st-srs, add comments to denote which #if to match.

Suggested change

#endif

#endif /* MD_CACHE_STACK */

I would suggest add comments in this way.

trunk/3rdparty/st-srs/stk.c

trunk/src/app/srs_app_server.cpp

suzp1984 · 2024-06-28T03:41:58Z

trunk/3rdparty/st-srs/stk.c

+#endif
+
+    extra = _st_randomize_stacks ? _ST_PAGE_SIZE : 0;
+#ifndef MD_CACHE_STACK


if don't cache stack, why not free _st_stack_t in _st_stack_free, don't use _st_free_stacks to store the freed stack?
The stack still not freed in _st_free_stacks, else a new coroutine started and request a new stack here.

The crash appears to occur when the st is freed within the _st_stack_free function, which seems to be happening while the stack is still in use. Although it is added to the _st_free_stacks structure, and the previous st is released before the next coroutine starts, this approach should not result in a significant memory leak issue.

Won't fix.

TRANS_BY_GPT4

winlinvip added the EnglishNative This issue is conveyed exclusively in English. label Jun 21, 2024

winlinvip added 2 commits June 21, 2024 18:07

SmartPtr: Support detect memory leak by valgrind.

30eded4

Fix build fail for thread local debug stat.

adcfb37

winlinvip force-pushed the feature/smart-ptr-valgrind branch from 9476ab2 to adcfb37 Compare June 21, 2024 10:07

winlinvip added 2 commits June 24, 2024 12:34

SmartPtr: Free the http handler entry.

69f1998

SmartPtr: Free the stack of ST.

ca3eaf1

winlinvip marked this pull request as ready for review June 27, 2024 11:09

winlinvip requested review from duiniuluantanqin and suzp1984 June 27, 2024 11:10

suzp1984 reviewed Jun 27, 2024

View reviewed changes

trunk/3rdparty/st-srs/stk.c Show resolved Hide resolved

suzp1984 reviewed Jun 27, 2024

View reviewed changes

trunk/src/app/srs_app_server.cpp Outdated Show resolved Hide resolved

suzp1984 reviewed Jun 28, 2024

View reviewed changes

winlinvip added 2 commits June 29, 2024 11:13

Fix bugs.

ff9b0fb

Update release to v6.0.132

3efecaa

winlinvip changed the title ~~SmartPtr: Support detect memory leak by valgrind.~~ SmartPtr: Support detect memory leak by valgrind. v6.0.132 Jun 29, 2024

winlinvip added the RefinedByAI Refined by AI/GPT. label Jun 29, 2024

winlinvip merged commit 7ab012c into ossrs:develop Jun 29, 2024

suzp1984 mentioned this pull request Jul 11, 2024

WIP: debug segment fault problem when SRS quit. #4114

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

SmartPtr: Support detect memory leak by valgrind. v6.0.132#4102
winlinvip merged 6 commits intoossrs:developfrom
winlinvip:feature/smart-ptr-valgrind

winlinvip commented Jun 21, 2024 •

edited

Loading

Uh oh!

suzp1984 Jun 27, 2024

Uh oh!

winlinvip Jun 29, 2024 •

edited

Loading

Uh oh!

suzp1984 commented Jun 27, 2024 •

edited

Loading

Uh oh!

suzp1984 Jun 27, 2024

Uh oh!

suzp1984 Jun 27, 2024

Uh oh!

winlinvip Jun 29, 2024

Uh oh!

suzp1984 commented Jun 27, 2024

Uh oh!

suzp1984 Jun 27, 2024

Uh oh!

Uh oh!

Uh oh!

suzp1984 Jun 28, 2024

Uh oh!

winlinvip Jun 28, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

	# Patched ST from https://github.com/ossrs/state-threads/tree/srs
	if [[ -f ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st/libst.a ]]; then
	rm -rf ${SRS_OBJS}/st && cp -rf ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st ${SRS_OBJS}/ &&
	echo "The state-threads is ok."
	else
	echo "Building state-threads." &&
	rm -rf ${SRS_OBJS}/${SRS_PLATFORM}/st-srs ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st ${SRS_OBJS}/st &&
	cp -rf ${SRS_WORKDIR}/3rdparty/st-srs ${SRS_OBJS}/${SRS_PLATFORM}/ &&
	env EXTRA_CFLAGS="${_ST_EXTRA_CFLAGS}" make -C ${SRS_OBJS}/${SRS_PLATFORM}/st-srs ${_ST_MAKE_ARGS} &&
	mkdir -p ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st &&
	cp -f ${SRS_OBJS}/${SRS_PLATFORM}/st-srs/${_ST_OBJ}/st.h ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st/ &&
	cp -f ${SRS_OBJS}/${SRS_PLATFORM}/st-srs/${_ST_OBJ}/libst.a ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st/ &&
	cp -rf ${SRS_OBJS}/${SRS_PLATFORM}/3rdparty/st ${SRS_OBJS}/ &&
	echo "The state-threads is ok."
	fi

Uh oh!

Conversation

winlinvip commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suzp1984 Jun 27, 2024

Choose a reason for hiding this comment

Uh oh!

winlinvip Jun 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suzp1984 commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suzp1984 Jun 27, 2024

Choose a reason for hiding this comment

Uh oh!

suzp1984 Jun 27, 2024

Choose a reason for hiding this comment

Uh oh!

winlinvip Jun 29, 2024

Choose a reason for hiding this comment

Uh oh!

suzp1984 commented Jun 27, 2024

Uh oh!

suzp1984 Jun 27, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

suzp1984 Jun 28, 2024

Choose a reason for hiding this comment

Uh oh!

winlinvip Jun 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

winlinvip commented Jun 21, 2024 •

edited

Loading

winlinvip Jun 29, 2024 •

edited

Loading

suzp1984 commented Jun 27, 2024 •

edited

Loading

winlinvip Jun 28, 2024 •

edited

Loading