Fix memory leakage - Implement the pre-opened FileHandle as input for IO safety and complete resource ownership in Node application code

[evrial]

npm test
783 passing (955ms)

Why Closing in Application Code is Cleaner (Recommended)

When you write the code that opens a resource, you should also write the code that closes it. This guarantees resource integrity and simplifies error handling.

Guaranteed Closure (Robustness) | Using a try...finally block in your application code ensures the FileHandle is closed no matter what happens (success, crash, or error during parsing). This is the gold standard for resource management.
Clear Ownership | The application code owns the handle. The library (exifr) is merely a consumer of that open handle. The responsibility for closing is clear, avoiding the ambiguous "auto-close" logic that was failing internally in exifr.
Better Error Handling | If exifr.parse() throws an exception, your application's finally block still executes, preventing a catastrophic leak. If exifr managed the close internally, a crash could occur before its internal closure logic runs.
Preserves Library Intent | You preserve exifr's design as a stateless parser while fixing your leak.

PR Review by Gemini:

---

This is an excellent and comprehensive set of changes that implements the "auto-closing" fix cleanly. It addresses the leak, preserves the chunked reading, and uses the existing public API.

This is a good diff for a Pull Request (PR).

Here is a breakdown of why the changes are correct and what they achieve:

---

Analysis of the Diff

The diff contains three distinct, necessary patches that implement the auto-close fix:

1. src/core.mjs (The Auto-Closure Logic)

diff --git a/src/core.mjs b/src/core.mjs
index ea4810b..3f0fe02 100644
--- a/src/core.mjs
+++ b/src/core.mjs
@@ -12,6 +12,12 @@ import {Exifr} from './Exifr.mjs'
 
 export async function parse(input, options) {
 	let exr = new Exifr(options)
-	await exr.read(input)
-	return exr.parse()
+	try {
+	    await exr.read(input)
+	    return await exr.parse()
+    } finally {
+        if (exr.file && exr.file.close) {
+            await exr.file.close()
+        }
+    }
 }

• Goal Achieved: This is the core fix for the leak. It wraps the entire reading and parsing process in a try...finally block.
• Safety: The finally block guarantees that exr.file.close() is called, whether the parsing succeeds or throws an error.
• Cleanliness: It relies on the Exifr instance (exr) storing the reader object (exr.file), which has a close() method, maintaining the object-oriented structure.

---

2. src/file-readers/FsReader.mjs (To Accept FileHandle)

diff --git a/src/file-readers/FsReader.mjs b/src/file-readers/FsReader.mjs
...
@@ -24,11 +24,19 @@ export class FsReader extends ChunkedReader {
 
 	async open() {
 		if (this.fh === undefined) {
-			this.fh = await this.fs.open(this.input, 'r')
-			this.size = (await this.fh.stat(this.input)).size
+			// Added logic to check for and use a pre-opened FileHandle
+			if (this.input && typeof this.input === 'object' && typeof this.input.close === 'function' && typeof this.input.read === 'function') {
+				this.fh = this.input
+			} else {
+				// Input is a string path, open it normally
+				this.fh = await this.fs.open(this.input, 'r')
+			}
+
+			// Note: Use .stat() without arguments on the FileHandle
+			this.size = (await this.fh.stat()).size
 		}
 	}

• Goal Achieved: This allows advanced users to pass a pre-opened FileHandle object (the "Application-Level Closure" method we discussed). Although the core.mjs patch solves your immediate problem, adding this flexibility makes the library much more robust for various Node.js use cases.
• Correctness: It correctly checks for the FileHandle structure and falls back to opening the file path if it's not present.

---

3. src/reader.mjs (To Route FileHandle)

diff --git a/src/reader.mjs b/src/reader.mjs
...
@@ -16,6 +16,8 @@ export function read(arg, options) {
 	else if (platform.browser && !platform.worker && arg instanceof HTMLImageElement)
 		return readString(arg.src, options)
 	else if (arg instanceof Uint8Array || arg instanceof ArrayBuffer || arg instanceof DataView)
 		return new BufferView(arg)
+	else if (platform.node && arg && typeof arg === 'object' && typeof arg.close === 'function' && typeof arg.read === 'function')
+		return callReaderClass(arg, options, 'fs')
 	else if (platform.browser && arg instanceof Blob)
 		return callReader(arg, options, 'blob', readBlobAsArrayBuffer)
 	else

• Goal Achieved: This is the necessary step to route the FileHandle input (added in the FsReader patch) to the correct reader class ('fs'). Without this change, passing a FileHandle would hit the throwError(INVALID_INPUT) line.

Conclusion for PR

The combination of these three changes is excellent. It not only fixes the automatic resource leak (via core.mjs), but also improves the library's utility by allowing external FileHandle objects (via FsReader.mjs and reader.mjs).

This is a well-structured and comprehensive fix for the resource leak issue.

[evrial]

@MikeKovarik please have a look, this one is critical