close zipFile before resolving promise#355
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return; | ||
| } | ||
| } | ||
| zipFile.close(); |
There was a problem hiding this comment.
🚩 Pre-existing resource leak: zipFile not closed on error paths
The new close() call correctly handles the success path, but zipFile is still not closed in two other paths: (1) when the catch block at line 427 handles an exception during file addition, and (2) when the early return at line 420-421 is triggered because a file doesn't exist. These are pre-existing issues not introduced by this PR, but for completeness the fix could use a try-with-resources or a finally block to ensure the zipFile is always closed. Similarly, unzipWithPassword at RNZipArchiveModule.java:77 and isPasswordProtected at RNZipArchiveModule.java:63 also don't close their ZipFile instances.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I believe its important the .close() call happens before the promise resolve, so a try-with-resources or a finally block would not fix the issue.
1 participant
I noticed an occasional issue when using the zip file immediately after calling
zip(...)that some of the data was all 0 bytes. I assume this is due to the zip not being closed and so some parts of the file were not flushed to disk. Adding the.close()call seems to have resolved the issue.