`IonFactory` leaks stream when parser/generator construction fails for `File`/`Path`

[cowtowncoder]

IonFactory extends DecorableTSFactory directly (it can use either textual or binary format), so it does not inherit the File/Path create methods from TextualTSFactory/BinaryTSFactory — and therefore not the leak fix from FasterXML/jackson-core#1693 either. Its own copies open the stream and hand it off with no try/catch:

// IonFactory.java:252
public JsonParser createParser(ObjectReadContext readCtxt, File f) {
    final InputStream in = _fileInputStream(f);
    IOContext ioCtxt = _createContext(_createContentReference(f), true);
    return _createParser(readCtxt, ioCtxt, _decorate(ioCtxt, in));
}

Anything that throws after the open — a user InputDecorator, or _createParser itself — leaks the file descriptor. Also note _fileInputStream(f) runs before _createContext(...), so a failure in _createContext leaks too; jackson-core opens the stream after.

Affects createParser(File) (:252), createParser(Path) (:260), and on the generator side createGenerator(File, JsonEncoding) (:358) and createGenerator(Path, JsonEncoding) (:366), which are the equivalent of FasterXML/jackson-core#1692.

Fix is the same shape as jackson-core: create the IOContext first, then open inside a try, and route RuntimeException through DecorableTSFactory._closeOnFailedConstruction(Closeable, RuntimeException) — protected as of jackson-core 3.1, so IonFactory can call it directly.

Every other backend (Smile, CBOR, Avro, Protobuf, XML, YAML, CSV) inherits the base-class methods and is already covered; Ion is the only one that opts out.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XrYRabZTvG9TjdXTCghvaM

[cowtowncoder]

Widening this slightly: IonFactory misses FasterXML/jackson-core#1694 as well, for the same reason.

That PR wrapped the base-class createParser/createGenerator methods so a failed construction releases the IOContext, returning the BufferRecycler lease to the pool instead of dropping it. Since IonFactory overrides those four File/Path methods itself, it inherits neither fix — origin/3.1 has zero occurrences of either helper in that file:

$ git grep -c "_releaseOnFailedConstruction\|_closeOnFailedConstruction" origin/3.1 -- ion/.../IonFactory.java
0

So each of the four methods wants the same catch block the base classes now use:

} catch (RuntimeException e) {
    _closeOnFailedConstruction(in, e);        // or `out` on the generator side
    _releaseOnFailedConstruction(ioCtxt, e);
    throw e;
}

with the source/target opened inside the try, after _createContext(...).

[Dongnyoung]

@cowtowncoder I'd be happy to work on this if no one else is already working on it. Would that be okay?

[cowtowncoder]

@Dongnyoung sure!

[Dongnyoung]

@cowtowncoder Thanks! I started looking into this and noticed one related lifecycle detail in the parser path.

createParser(File/Path) creates an IOContext for decoration, but _createParser(..., InputStream) later replaces it with a new IOContext created from the IonReader:

IonReader ion = _system.newReader(in);
ioCtxt = _createContext(_createContentReference(ion), true);

I checked #325 and understand that the second IOContext is intentional so that the IonReader is resource-managed and closed with the parser.

However, the first IOContext also acquires its own BufferRecycler, and after the replacement it looks like that context is no longer owned by the parser and may not be released even on the successful construction path.

This seems related, but slightly broader than the failed-construction cleanup in #780, since it may also affect the successful construction path and other parser overloads that go through the same helper.

To keep #780 focused on failed-construction cleanup, I opened a separate issue to track this.