Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memleak when emptying session history and using session-based execute callbacks. #171

Closed
crouchingtiger opened this issue Oct 6, 2021 · 4 comments
Assignees
Labels
android Affect Android platform apple Affect Apple platforms bug Something isn't working fixed-in-v4.5.1 library Affects the library v4.5 Affects v4.5 release

Comments

@crouchingtiger
Copy link

crouchingtiger commented Oct 6, 2021

When calling [FFmpegKitConfig clearSessions], only the sessionHistoryList is emptied, leaving sessionHistoryMap to leak and retain a reference to the FFmpegSession.

Sessions history list and map are added at https://github.com/tanersener/ffmpeg-kit/blob/5d11e79fc2734a7c61b741f3a729409a17b212e1/apple/src/FFmpegKitConfig.m#L115

Notably, if you are using session execute callbacks , ffmpeg-kit will continue to leak FFmpegSession objects, even if you set [FFmpegKitConfig setSessionHistorySize:0]. I've noticed this behaviour in previous versions but didn't investigate further at the time and was happy enough to use the global callbacks. My use-case has shifted and I have concurrent ffmpeg sessions running for my application, so I took a further look. This affects asynchronous commands issued on another queue. lldb will not be able to point out any obvious leaks here.

As a test, I issued a sample command to + (FFmpegSession*)executeAsync:(NSString*)command withExecuteCallback:(ExecuteCallback)executeCallback withLogCallback:(LogCallback)logCallback withStatisticsCallback:(StatisticsCallback)statisticsCallback onDispatchQueue:(dispatch_queue_t)queue with non-null, empty blocks. I monitored and waited for the autoreleasepool and took some samples after successful return.

Here's the situation without calling the newly added [FFmpegKitConfig clearSessions]:

Bildschirmfoto 2021-10-06 um 17 53 34

and if you do decide to clear the session list:

Bildschirmfoto 2021-10-06 um 17 45 06

Only if ([sessionHistoryList count] > sessionHistorySize) are entries removed when adding a new session. This means at least 1 session will always linger and leak, replacing the previous reference.

Based off this commit, I added a method call to clear out sessionHistoryMap. I can confirm that in my use-case, my leaks disappear once I call [FFmpegKitConfig clearSessions].

Environment

Platform: [macOS]
Architecture: [x86_64]
Version [v4.5]
Source branch [master, v4.5]
Xcode version [13.0]

@tanersener
Copy link
Collaborator

tanersener commented Oct 6, 2021

Good catch! Thanks for reporting this. It looks like we need to patch clearSessions method. This affects Android too.

@tanersener tanersener added android Affect Android platform apple Affect Apple platforms bug Something isn't working library Affects the library v4.5 Affects v4.5 release labels Oct 6, 2021
@tanersener tanersener self-assigned this Oct 6, 2021
@tanersener tanersener added the fixed-on-development Fixed on the development branch. Not released yet. label Oct 7, 2021
@tanersener
Copy link
Collaborator

Fixed in the development branch.

@crouchingtiger
Copy link
Author

@tanersener Thanks for the quick response and fix (and project)!

@arthenica arthenica deleted a comment from github-actions bot Dec 7, 2021
@tanersener tanersener added fixed-in-v4.5.1 and removed fixed-on-development Fixed on the development branch. Not released yet. labels Dec 31, 2021
@tanersener
Copy link
Collaborator

The fix is released in v4.5.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android Affect Android platform apple Affect Apple platforms bug Something isn't working fixed-in-v4.5.1 library Affects the library v4.5 Affects v4.5 release
Projects
None yet
Development

No branches or pull requests

2 participants