ConfigJSR icon indicating copy to clipboard operation
ConfigJSR copied to clipboard

Improve autoclosable doc

Open Emily-Jiang opened this issue 7 years ago • 6 comments

add an example add more explanation

Emily-Jiang avatar Jan 11 '18 15:01 Emily-Jiang

A typical ConfigSource example would be a ConfigSource that uses a DB to store its props. For performance reason, we want to reuse a JDBC connection and closes it when the ConfigSource is not longer used.

However we have edge cases when using the programmatic API as such a ConfigSource can belong to multiple Configs. If that's the case, from the ConfigSource implementation POV, I have no idea when my close method is called if there are other Configs that still uses it. And I can not use reference counter since I can't know how many Configs uses it.

I wonder if we shouldn't have a symmetrical lifecycle init methods. So that when a Config includes me, it calls my init() and when it releases me, it calls my close() This way, the ConfigSource could know when it is no longer used at all and be able to free its resources when the last Config has released it.

The same argument can also be made for Converter. For example, I have a GPSConverter that takes a GPS coordinates String and calls a DB to get the data and creates an Address object. This converter needs to hold stateful resources (the DB connections). It needs also to know when it can release them. As for ConfigSource, a Converter can be hold by many Configs (using the programmatic API).

I propose we add an empty default init() method to ConfigSource and Converter API. These methods will always be called when a ConfigSource or a Converter are added to a Config.

jmesnil avatar Jan 11 '18 16:01 jmesnil

I wonder it may be implementation issue. If an implementation allow ConfigSource to be shared by multiple Config, can this implementation handle the edge case by their own way ?

kazumura avatar Jan 18 '18 09:01 kazumura

The spec allows (or at least does not forbid) to share ConfigSources between Config using the programmatic ConfigBuilder.

The actual sentence in the spec is not correct:

If a ConfigSource implements the java.lang.AutoCloseable interface then the close() method will be called when the underlying Config is being released.

There is no Config under an ConfigSource (that's the other way around). The relation are:

  • 1 Config has many ConfigSource
  • 1 ConfigSource belongs to many Config

I'd be fine with requiring Config implementation to manage this case but it should be explicitly stated in the spec something like:

If a ConfigSource implements the java.lang.AutoCloseable interface then its close() method will be called when the last Config that provides it is released.

jmesnil avatar Jan 18 '18 13:01 jmesnil

The more we talk about this, the more I think the close is implementation detail. Maybe we just mention in the spec that the implementors need to ensure when a config object is released, the underline config source or converters should be cleared up.

Emily-Jiang avatar Jan 18 '18 14:01 Emily-Jiang

@Emily-Jiang Do you think we should still add any additional clarification or hint to the spec? Or do you (also) think we are good now and whoever implements such edge cases has to deal with them himself?

struberg avatar Jan 25 '18 15:01 struberg

I think it will be good to add additional clarification.

Emily-Jiang avatar Jan 26 '18 10:01 Emily-Jiang