awslabs / amazon-kinesis-video-streams-webrtc-sdk-c

Amazon Kinesis Video Streams Webrtc SDK is for developers to install and customize realtime communication between devices and enable secure streaming of video, audio to Kinesis Video Streams.
https://awslabs.github.io/amazon-kinesis-video-streams-webrtc-sdk-c/group__PublicMemberFunctions.html
Apache License 2.0
995 stars 298 forks source link

[Bug]: Memory leak for WebRTC SDK 1.10.2 when testing sample code #1986

Closed xiaomizhouya closed 1 day ago

xiaomizhouya commented 1 month ago

Please confirm you have already done the following

Please answer the following prompt

Describe the bug

Hi, I encountered a memory leak issue while testing the sample code of WebRTC SDK 1.10.2. The specific details are as follows

  1. Only replace the thingName, accessKey, secretKey, session Token, and cacert in the sample code with the device's own, without making any changes to the other codes.
  2. Use the test webpage to view the live view( https://awslabs.github.io/amazon-kinesis-video-streams-webrtc-sdk-js/examples/index.html
  3. View the memory changes of the webrtc process through the “top” When the webrtc process runs, the memory is 5556KB, The first time I watched a live view, my memory increased from 5556KB to 11304KB. When I stopped playing, my memory decreased from 11304KB to 9932KB. The second time watching live view, the memory increased from 9932KB to 12652KB. When playback stopped, the memory decreased from 12652KB to 12632KB The third time watching live view, the memory increased from 12632KB to 14748KB. When playback stopped, the memory decreased from 14748KB to 14664KB

The attachment provides log information, which can quickly locate the corresponding position of the log based on the above data. Please help confirm if there is any issue with the SDK that I described, as I did not encounter this issue in WebRTC SDK 1.7.3. Serial-COM17-05-15-16-48.log 20240515171556

Expected Behavior

Try to verify the issue i mentioned and if exist, fix it.

Current Behavior

n/a

Reproduction Steps

n/a

WebRTC C SDK version being used

1.10.2

If it was working in a previous version, which one?

No response

Compiler and Version used

gcc 7.4.0

Operating System and version

ubuntu 18.04

Platform being used

linux

naseebpanghal commented 1 month ago

Also, I find heap usage is growing unexpectedly. Attaching video which shows, client is receiving the stream but heap keeps growing. I performed test on ARM device.

https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/assets/24919381/19083efc-a20e-4c0a-8d63-ca41a157481c

naseebpanghal commented 1 month ago

One more observation, heap size becomes stable after holding lot of space. I am wondering why heap keeps growing. On my Intel + ubuntu(22.04) with 32 RAM, it works fine.

disa6302 commented 1 month ago

@naseebpanghal ,

Thank you for the details! The memory is expected to grow for a few min after which it stablizes. Could you also confirm if there are any sample changes you have made or you ran a clean build post upgrading and then ran the sample?

naseebpanghal commented 1 month ago

@disa6302 , Thanks for the prompt response.

The sample is intact. I have just used a printf statement to print video file index being sent. when I run sample on Intel machine with 32 GB RAM, there heap doesn't grow as it grows for embedded device. NOTE: I am monitoring heap with /proc/pid/smaps file.

Is there any buffer pool being maintained internally? I don't have much memory on embedded device. Is there any configuration which can to be used to avoid this heap growth which happen for few minutes.

hassanctech commented 1 month ago

@naseebpanghal @xiaomizhouya

Can you please try with cmake flag ENABLE_KVS_THREADPOOL OFF? https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/blob/master/CMakeLists.txt#L21

naseebpanghal commented 1 month ago

@hassanctech ,

I have switched ENABLE_KVS_THREADPOOL to OFF as mentioned, but still lots of thread gets created and I find only one thread is working and other are resting in process address space. FYI, I have same effect after disabling the thread pool flag as well.

xiaomizhouya commented 1 month ago

@hassanctech ,

According to your suggestion, there was no significant improvement in turning ENABLE_KVS_THREADPOOL to OFF. The actual memory of the WebRTC process is still growing significantly, and after live view, RSS will not decrease to the level before live view.

xiaomizhouya commented 1 month ago

@hassanctech , Hi, have you tested the above issues? If so, how long will it take to solve them and release a new version of SDK?

hassanctech commented 1 month ago

@xiaomizhouya I have not been able to reproduce a regression between this version and a previous version, I will report back once I have some new information.

xiaomizhouya commented 1 month ago

@hassanctech Hi, Can you guys reproduce or make any progress on this issue? This issue is important to us.

hassanctech commented 1 month ago

@xiaomizhouya We have made significant improvements for memory usage in our develop branch which we hope to release in the next month or so but do not have a date yet. Can you please try this you should see a significant improvement overall in memory: https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/tree/develop

niyatim23 commented 1 month ago

Hi @xiaomizhouya @naseebpanghal, I ran the latest release with valgrind and tested connecting and disconnecting from the JS viewer multiple times like you've suggested. I don't see a leak overall. It could be a possibility that something may not have been freed up until the end of the application:

==807792== 
==807792== HEAP SUMMARY:
==807792==     in use at exit: 0 bytes in 0 blocks
==807792==   total heap usage: 227 allocs, 227 frees, 21,159 bytes allocated
==807792== 
==807792== All heap blocks were freed -- no leaks are possible
==807792== 
==807792== For lists of detected and suppressed errors, rerun with: -s
==807792== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
xiaomizhouya commented 1 month ago

@niyatim23 May I ask if you have observed the RSS of the webrtc process? In the testing steps I provided, after live view, the RSS of the webrtc process cannot return to the previous value. Moreover, based on the testing results I provided, after checking the live view three times, the RSS value increased from 5556KB to 14664KB, which seems unreasonable. Can you check the RSS value of the webrtc process during your testing? Will it continue to increase? Not just using the valgrind tool.

xiaomizhouya commented 1 month ago

@hassanctech I noticed that there have been new changes in the develop branch recently: [Fix MID-NTP macro definition]. Does this solve the memory leak issue? If so, I will conduct further testing on my end

niyatim23 commented 1 month ago

Hi @xiaomizhouya, I don't think that is related to this issue. We are actively investigating this issue, we will get back to you once we have something. Meanwhile, can you check if you can use valgrind for different stages of the application demonstrating the leak?

naseebpanghal commented 1 month ago

@niyatim23 @hassanctech @disa6302

I have done some changes to restrict memory usage within RTP rolling buffer, within the application. Below is the patch

-#define DEFAULT_ROLLING_BUFFER_DURATION_IN_SECONDS 3
-#define HIGHEST_EXPECTED_BIT_RATE                  (10 * 1024 * 1024)
+#define DEFAULT_ROLLING_BUFFER_DURATION_IN_SECONDS 1
+#define HIGHEST_EXPECTED_BIT_RATE                  (2 * 1024 * 1024)

Although with above changes, I don't find any issue so far.

  1. But I want to understand what impact it can bring?
  2. Should I handle anything alongwith these changes?

@xiaomizhouya You can also try above change your side. I hope that may help you.

disa6302 commented 1 month ago

@xiaomizhouya ,

The develop branch has multiple changes that help with memory reduction, would suggest you clean build try that out.

@naseebpanghal ,

These settings are according to your requirements. The buffer defines you changed affect the max rtp packets that can live in memory at a time. The develop branch makes this configurable in the application, so you would not have to change this value in the source

naseebpanghal commented 3 weeks ago

@disa6302

The develop branch makes this configurable in the application, so you would not have to change this value in the source For above statement, please let me know how to apply configuration during compile time of my application. Which CMakefile has these configurations.

niyatim23 commented 3 weeks ago

Hi @naseebpanghal, @xiaomizhouya, the heap memory is functioning as expected. The memory increase is due to VM: Stack. The memory drops in the heap below mark the disconnection from the viewer and the increases mark connection

Screenshot 2024-06-11 at 7 50 47 AM
xiaomizhouya commented 3 weeks ago

@disa6302 I encountered an error during the compilation process while testing the develop branch, with a prompt that I couldn't find header files such as "kvsrtp/rtp-api.h" and "kvssdp/sdp-data_types. h". Since I didn't automatically compile the develop branch, but rather added the header file path in the makefile, I want to know if the develop branch has added some dependency libraries or if the previously dependent libraries need to be updated?

xiaomizhouya commented 3 weeks ago

@naseebpanghal Thank you very much for your suggestion. I tested according to your method and did indeed slow down the rate of memory increase. However, I found another issue. When multiple people simultaneously view the live view, memory cannot return to its previous level when exiting. It seems that memory is still increasing. Have you encountered this situation on your end?

naseebpanghal commented 3 weeks ago

@niyatim23 @disa6302 @hassanctech

-#define DEFAULT_ROLLING_BUFFER_DURATION_IN_SECONDS 3 -#define HIGHEST_EXPECTED_BIT_RATE (10 1024 1024) +#define DEFAULT_ROLLING_BUFFER_DURATION_IN_SECONDS 1 +#define HIGHEST_EXPECTED_BIT_RATE (2 1024 1024)

I have gone through multiple CMakefiles but don't find a way to make above changes configurables. My concern, If I make above changes local, then future kvs releases will overwrite my changes and every time I need correct them.

Suggest anyway, compile time or runtime, i can use above settings without making any change in your source files. OR is there any plan to make these Macros configurable through CMakfiles?

@niyatim23 One more thing what is VM: Stack (Is it related to RTP Rolling buffer?)

@xiaomizhouya Sorry, as of now, I am testing single stream.

disa6302 commented 3 weeks ago

@naseebpanghal ,

https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/tree/develop?tab=readme-ov-file#controlling-rtp-rolling-buffer-capacity

@xiaomizhouya , For your change, did you clean build? Yes there are some new libraries that have been added. These need to be linked to the webrtc client and signaling libraries if you have modified the cmake files that come with this repo. This is where the linking happens for 3 new libraries:

https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/blob/develop/CMakeLists.txt#L412

https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/blob/develop/CMakeLists.txt#L436

niyatim23 commented 3 weeks ago

@xiaomizhouya, @naseebpanghal, I ran v1.7.3 and v1.10.2 on a Linux and MacOS both. Following are my observations

  1. Linux

    • Observed the memory using htop and /proc/$PID/smaps

    • v1.7.3

      • 
        ===== Signaling =====
        
        VmPeak:   238352 kB
        VmSize:   172816 kB
        
        Resident size (kb): 7680
        
        ===== Viewer connection =====
        
        VmPeak:   634780 kB
        VmSize:   568844 kB
        
        Resident size (kb): 14208
        
        ===== Viewer disconnection =====
        
        VmPeak:   634780 kB
        VmSize:   557868 kB
        
        Resident size (kb): 11580
        
        ===== Viewer connection =====
        
        VmPeak:   926592 kB
        VmSize:   779064 kB
        
        Resident size (kb): 14396
        
        ===== Viewer disconnection =====
        
        VmPeak:   926592 kB
        VmSize:   754476 kB
        
        Resident size (kb): 13368            
    • v1.10.2

      • 
        ===== Signaling =====
        
        VmPeak:  254460 kB
        VmSize:  254460 kB
        
        Resident size (kb): 7808
        
        ===== Viewer connection =====
        
        VmPeak: 1068332 kB
        VmSize: 1043744 kB
        
        Resident size (kb): 15016
        
        ===== Viewer disconnection =====
        
        VmPeak: 1068332 kB
        VmSize: 1032728 kB
        
        Resident size (kb): 12344
        
        ===== Viewer connection =====
        
        VmPeak: 1270316 kB
        VmSize: 1188388 kB
        
        Resident size (kb): 16440
        
        ===== Viewer disconnection =====
        
        VmPeak: 1270316 kB
        VmSize: 1163800 kB
        
        Resident size (kb): 15264
  2. MacOS
    • Used Instruments to observe the heap and stack
    • v1.7.3
      • Screenshot 2024-06-11 at 7 36 56 AM
    • v1.10.2
      • Screenshot 2024-06-11 at 7 50 47 AM

No issues reported by valgrind as pointed out earlier. The memory continues to increase after connection to the viewer but stops after a point. We also see it drop after a disconnection but not back to what it was right after signaling. However, we do see it dropping back to where it started from in heap allocations in the MacOS instruments plots.

This behavior is seen in both v1.7.3 and v1.10.2. Possible reasons for this delay in releasing the VM Stack maybe that the OS may be delaying the release of memory back to the system to avoid the overhead associated with frequent memory allocation and deallocation. Or the virtual memory manager might not immediately release memory back to the system, especially if the system is not under memory pressure. The memory pages can remain in the working set or be swapped out and marked as available for future allocations.

github-actions[bot] commented 2 weeks ago

It looks like this issue has not been active for 10 days. If the issue is not resolved, please add an update to the ticket, else it will be automatically resolved in a few days.

xiaomizhouya commented 1 week ago

1

I tested the sample code of webrtc 1.10.2 sdk on my device (linux system) and monitored the memory of webrtc using the top command. As shown in the statistical chart, the vertical axis represents the RSS value in kb, the horizontal axis represents the number of points, and the interval between points is 5 seconds (the top command refresh time is 5 seconds). There are a total of 4033 points in the chart, so the testing time is 4033 * 5s/3600s=5.57 hours.

The testing method is to observe the live view for 60 seconds, then stop for 15 seconds, and repeat this process. Use only one peer to view the live view throughout the entire process.

Is the curve shown in the graph normal?

niyatim23 commented 1 week ago

RSS memory includes heap, stack, code, data, shared libraries, memory-mapped files. It isn't just the heap. We've already established that the heap memory goes back to what it was before any connections after the disconnection and there are no leaks related to it. We have also established that the total memory for a particular peer connection becomes steady after a point which can also be seen in your plot. The VM Stack may not have been claimed back by the OS which may have been causing the increase. I do see some fluctuation around ~2300. What is that drop related to? Do you see a similar pattern on v1.7.3 as seen on v1.10.2?

github-actions[bot] commented 5 days ago

It looks like this issue has not been active for 10 days. If the issue is not resolved, please add an update to the ticket, else it will be automatically resolved in a few days.