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

Fix android leaks #2141

Merged

Conversation

renchap
Copy link
Contributor

@renchap renchap commented Jun 8, 2023

Motivation (required)

While working on solving #2138, @huextrat noticed that the various streams and descriptors were never closed.

This is leaking a lot of resources.

I converted those to try-with-resources syntax, so they are automatically closed after the block ends.

Test Plan (required)

I tested various files and videos with the example app, and did not notice any issue.

I also enabled StrictMode on the example app (see https://wh0.github.io/2020/08/12/closeguard.html) and it no longer detects non-closed resources.

@renchap
Copy link
Contributor Author

renchap commented Jun 8, 2023

My last commit also ensure that MediaMetadataRetriever is closed, this seems to heavily reduce the ANRs I was experiencing in the linked issue.

@renchap
Copy link
Contributor Author

renchap commented Jun 8, 2023

This has now been tested on Android 9 as well.

MediaMetadataRetriever does not implements AutoCloseable on API < 29, so I added a custom wrapper.

I also removed my commit replacing the byte-copy loops with InputStream.transferTo() as this somehow does not work on Android 9 (code execution stops there and the JS Promise never returns).

Johan-dutoit
Johan-dutoit previously approved these changes Jun 12, 2023
@Johan-dutoit
Copy link
Collaborator

Johan-dutoit commented Jun 12, 2023

@renchap thanks for the changes! Please resolve conflicts. Will review/merge then.

Streams and file descriptors needs to be closed when no longer used.

I converted those to try-with-resources syntax, so they are
automatically closed after the block ends.
@Johan-dutoit Johan-dutoit merged commit 8e4b205 into react-native-image-picker:main Jun 15, 2023
@renchap renchap deleted the fix-android-leaks branch June 15, 2023 08:33
@Johan-dutoit
Copy link
Collaborator

🎉 This PR is included in version 5.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants