openpreserve / nanite

Nanite - a friendly swarm of format-identifying robots.
openplanets.github.io/nanite/
15 stars 13 forks source link

Fix memory leak #46

Closed sparkhi closed 7 months ago

sparkhi commented 7 months ago

1) Made use of the try-with-resource to close the request automatically. 2) The close method seem to remove the temp directory anyways, so the call to remove temp directory (request.removeTempDir();) in finally block became redundant, hence removed it.

anjackson commented 7 months ago

Thanks for taking the time to look at this @sparkhi and I'm sorry this was really down to my poor documentation, as per https://github.com/digital-preservation/droid/pull/1084#issuecomment-2060752180

I will look at improving the documentation.

sparkhi commented 7 months ago

No problem @anjackson The change had allowed me to run MXF files multiple times. This particular one that I closed is a local variable so I don't think there is any caller which can close this? Perhaps I misunderstood or there maybe another request that you were referring to.

anjackson commented 7 months ago

If I remember correctly, the issue is that this line registers the callers InputStream inside the object, wrapped as a Reader:

https://github.com/sparkhi/nanite/blob/fa6f7b1fa0c52c18e821f0dad16ccddc36548405/nanite-core/src/main/java/uk/gov/nationalarchives/droid/internal/api/DroidAPIExtended.java#L192

And closing the Reader will close that wrapped input stream.