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

[expo-video-thumbnails][Android] Fix memory leaks #26100

Merged
merged 2 commits into from
Dec 23, 2023
Merged

Conversation

hirbod
Copy link
Contributor

@hirbod hirbod commented Dec 23, 2023

Why

  1. Closing ParcelFileDescriptor properly: We were leaving the ParcelFileDescriptor open after use, which could lead to resource leaks
  2. Releasing MediaMetadataRetriever after use: Previously, we weren't releasing the MediaMetadataRetriever, potentially leading to memory leaks.

How

  1. I wrapped the ParcelFileDescriptor in a use block. This way, it automatically closes once we're done with it.
  2. Ensuring MediaMetadataRetriever release: I added a finally block in the GetThumbnail class. This block is executed after retrieving the frame, ensuring retriever.release() is called every time, which guarantees that the resources are freed up.

Test Plan

  • Emulator running SDK 33 and 34
  • Real device: Google Pixel Pro 7

Checklist

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Dec 23, 2023
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Dec 23, 2023
@hirbod hirbod marked this pull request as ready for review December 23, 2023 09:36
@hirbod hirbod requested a review from tsapeta as a code owner December 23, 2023 09:36
Copy link
Member

@tsapeta tsapeta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 😊

@tsapeta tsapeta merged commit c269779 into expo:main Dec 23, 2023
9 checks passed
@hirbod hirbod deleted the patch-2 branch December 23, 2023 11:13
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants