support IOContext closing in GeneratorBase

[pjfanning]

JsonGeneratorImpl closes the IOContext now (part of the BufferRecycler changes in v2.16).
Unfortunately, there are generators in other Jackson modules that now need to close the IOContext (eg TomlGenerator). It would be easier if GeneratorBase was changed and then classes like TomlGenerator could be updated to notify GeneratorBase about their IOContext and let GeneratorBase do closing.

ParserBase has the IOContext close in it so it is pretty strange not to support it in GeneratorBase.

wdyt @cowtowncoder @mariofusco ?

[mariofusco]

Good idea. I fully agree that the IOContext lifecycle should be managed at the highest possible level in the hierarchy of generators. Is there something similar that should be done also for parsers?

[pjfanning]

Good idea. I fully agree that the IOContext lifecycle should be managed at the highest possible level in the hierarchy of generators. Is there something similar that should be done also for parsers?

ParserBase already manages this. There is a ParserMinimalBase and it doesn't. I don't want to overcomplicate this PR but there might be some merit in also changing ParserMinimalBase in another PR.

[cowtowncoder]

Hmmh. I'll have to think about this. I agree it makes sense overall -- I think 3.0 had already demoted IOContext further down -- but am slightly concerned about backwards-compatibility with non-FasterXML backends (BSON, MsgPack).
Especially the case of defaulting to passing null IOContext.

But I'll think about this: will probably agree this is necessary.

[pjfanning]

Hmmh. I'll have to think about this. I agree it makes sense overall -- I think 3.0 had already demoted IOContext further down -- but am slightly concerned about backwards-compatibility with non-FasterXML backends (BSON, MsgPack). Especially the case of defaulting to passing null IOContext.

But I'll think about this: will probably agree this is necessary.

This is why in the main PR I was trying to argue that the IOContext changes were very damaging. We now have to go and fix all non-JSON parser and generator solutions. I had a look at jackson-dataformats-text today and we now have multiple parsers and generators that don't close their IOContexts.

This PR makes it a bit easier to fix this - because many of the generators extend GeneratorBase. We would still need to change the Generator impl to use the new constructors that take the IOContext as a param.

I can't just remove the old constructors for backward compatibility. I hate nulls at least as much as anyone else but there isn't really a good solution to supporting the deprecated constructors.

[pjfanning]

FasterXML/jackson-dataformats-text#427 doesn't rely on this change. That code explicitly closes the IOContext in the generators.

[cowtowncoder]

Right. At the same time, we had to add a life-cycle for recycling BufferRecyclers. I guess we still have the possibility of changing things to make parser/generator handle closing... but, since IOContext is deemed the owner of BufferRecyler that probably wouldn't be any cleaner.

So I think we'll go with it: as you point out, with the default recycler pool, closing really isn't necessary.

EDIT: and although 3.0 does have _ioContext in GeneratorBase that all impls share, ParserMinimalBase doesn't have it -- probably should.

[cowtowncoder]

Ok yeah, let's go with this.