search

sannies / mp4parser

A Java API to read, write and create MP4 files
Apache License 2.0
2.74k stars 563 forks source link

Fix some file descriptor / tmp file leaks #397

Closed chrisribble closed 2 years ago

chrisribble commented 4 years ago

Extend Closeable in Container and modify BasicContainer to close all child boxes when it is closed, giving all child boxes a chance to release any resources that they hold.

Prevent new IsoFile(String) and new IsoFile(File) from leaking file descriptors by implementing Closeable and ensuring that FileInputStream reference is stored and closed when IsoFile is closed.

Prevent MediaDataBox from exhausting space in /tmp by removing disk-buffered data file when IsoFile is closed.

Convert some stream definitions to use Java 7 try-with-resources syntax

Remove superfluous explicit toString invocations for string concatenation

chrisribble commented 3 years ago

@sannies any thoughts on this patch?

Fixing the various resource leaks makes the code more robust for production use cases.

It also makes it a lot easier to work with IsoFile in a process that runs for a long time (i.e. a web application), since a consumer only needs to ensure that it closes the IsoFile when it is done with it, versus needing to dig into the internal fields of the MediaDataBox using reflection so that it can delete the temporary files that are left behind.

bswami50 commented 3 years ago

+1 on integrating the patch, if possible. We are also running into this issue. This also seems to be the same issue: https://github.com/sannies/mp4parser/issues/315

chrisribble commented 3 years ago

Yes, I believe this is a reasonable fix for #315, at least until a better design is implemented for reading the mdat contents without copying the data into a separate file.

Fixing it more elegantly (by avoiding the file copy) was going to require quite a bit more thought, so I did this so that I could get rid of the hacky reflection code that I had written in my application to delete the files that the MediaDataBox was leaving behind.

chrisribble commented 2 years ago

@sannies I finally got around to making this patch a bit slimmer. Any interest in merging it?