AutoCloseable beans

[gavinking]

I've recently been trying out Spring for the first time, in order to get Hibernate Data Repositories working with it. To my surprise, Spring's support for dependency injection is on the whole quite inelegant and still, after all these years, lacking in type safety compared to CDI. It looks like at one point in history they basically just put a veneer of annotations over their old XML stuff, and then sorta left it at that. The annotations themselves are a real hodgepodge strewn about various packages, with quite bad naming, For example, their equivalent of @Produces is called @Bean and has seven weird and messy-looking members, most of them stringly-typed.

But there is so far one single thing I've run into that I liked, and that is missing in CDI.

It seems that by default Spring will automatically call the close() method of any bean which implements AutoCloseable. In CDI I would have two options:

1. declare a @PreDestroy method if it's a regular bean, or
2. write a @Disposes method if it's a producer method.

Option 1 is in general completely fine, and arguably @PreDestroy is easier than implementing AutoCloseable. But option 2 for producer methods is actually significantly worse, since it's very common that the object returned by a producer method is defined by a library or framework, and already implements AutoCloseable. For example, Hibernate's StatelessSession, or whatever.

I think there should be some way to indicate to CDI that it should treat the close() method as a @PreDestroy callback. Ideally this would be automatic, but that would break backward compatibility, so I guess we need some sort of annotation for that. I'm not sure of the best way to expose that, but perhaps something like this is OK, if not perfect:

@Produces @Closeable @TransactionScoped
StatelessSession statelessSession(SessionFactory factory) {
    return factory.openStatelessSesssion();
}

Does anyone have a better idea?

[manovotn]

So, this is basically a syntactic sugar, or a shortcut if you will, to achieve what's already there.
You could always just have the following:

@Produces @TransactionScoped
StatelessSession statelessSession(SessionFactory factory) {
    return factory.openStatelessSesssion();
}

 void close(@Disposes SessionFactory sf) {
        sf.close();
    }

... since it's very common that the object returned by a producer method is defined by a library or framework, and already implements AutoCloseable.

This assumption may or may not be true, there is plenty of beans that do not implement the interface.
Producers (or synth beans for that matter) are often just a way to circumvent bean creation restrictions that CDI imposes and it has nothing to do with having AutoCloseable or not.

...so I guess we need some sort of annotation for that.

Yea, that would work.
Then there are also synthetic beans but I am not quite sure it's worth the hassle there - defining the disposer would probably be just as efficient as adding a flag for using object's closeable call.

[Ladicek]

Yeah, I also think this seems reasonable. My biggest questions are:

1. What about synthetic beans? We can probably add a flag to consider them auto-closeable as well.
2. What about the ordering between AutoCloseable.close() and @PreDestroy/@Disposes? I'm not sure what order is best, but we probably should introduce one.

[gavinking]

You could always just have the following:

Correct, as I said, you can "write a @Disposes method". But that's at least a bit uglier.

This assumption may or may not be true, there is plenty of beans that do not implement the interface

Which is why I said "it's very common". I didn't say it's always the case.

there are also synthetic beans but I am not quite sure it's worth the hassle there

It would be nice if there were some way to opt into this via the API.

What about the ordering between AutoCloseable.close() and @PreDestroy/@Disposes?

Two reasonable options:

1. close() happens last, or
2. it's simply illegal to have both.

[gavinking]

I guess we need some sort of annotation for that.

If people object to a new annotation for such a small thing, a different option would be to make it a member of the @Produces annotation. But that has some disadvantages, and we've generally eschewed the use of annotation members in CDI.

[Ladicek]

I think an annotation is fine, although I wouldn't call it @Closeable just because there's already java.io.Closeable. So I'm thinking @AutoClose, maybe.

I also think this should apply to all kinds of beans, not just producers, although producers will probably benefit most.

[gavinking]

I think an annotation is fine, although I wouldn't call it @Closeable just because there's already java.io.Closeable. So I'm thinking @AutoClose, maybe.

Ick, good point.

[manovotn]

What about synthetic beans? We can probably add a flag to consider them auto-closeable as well.

In case of PE, its a one-liner for both so not much to gain there. Though I agree that in case of BCE, the flag might be convenient.

it's simply illegal to have both.

This is IMO the best approach - if you want to use closeable and perform some other custom action, just declare a disposer and invoke the close() as and when needed.

[Ladicek]

IIRC, class-based beans can have @PreDestroy on the target class and also inherit it from superclasses, so forbidding to have @PreDestroy and @AutoClose both at the same time may pose problems in API evolution (superclasses may gain AutoCloseable in the future). So I'd prefer to specify ordering instead of forbidding it.

[manovotn]

Hm, the inheritance aspect didn't strike me; you're right.

[gavinking]

Interestingly, you could define @AutoClose as a qualifier annotation, if you also had the following built-in disposer method:

void autoClose(@Disposes @AutoClose AutoCloseable bean) {
    bean.close();
}

[Ladicek]

Talked about this on the CDI call today and were mildly positive about this. The only question we have is: does @AutoClose mean the producer always produces an instance of a class that implements AutoCloseable, or does it mean the producer sometimes produces an instance of a class that implements AutoCloseable? I assumed it's the latter, @manovotn thought it's the former (which means we could validate the type and produce a definition error if it does not).