Re: [bv-dev] Do we need ConstraintValidatorFactory#releaseInstance()?

Thanks you for the sum up. This email is not a rebuttal, more a set of remarks on the subject. If we manage to remove `releaseInstance()` then we should also remove `ValidatorFactory.close()`. AFAIR, the only reason for `close()` to exist is to allow proper implementations of releaseInstance. The ConstraintValidatorFactory contract is a public contract that can be implemented by anyone. This is also the way for DI framework to provide injectable `ConstraintValidator` implementations. In my mind and in retrospect, `ConstraintValidatorFactory#getInstance()` should have been named `createInstance()`. Remember, caching is not allowed in a `CosntraintValidatorFactory` implementation. So you argument that what creates an object should destroy it applies here: what requests an object creation should request its destruction and that's exactly why we have added this method. libs (let's say DI frameworks) have a @PreDestroy logic that is called when the CVF bean would be destroyed by its creator. It seems though that the @PreDestroy method would have to process all objects returned by `getInstance()`. So `getInstance` would need to keep track of them in a concurrent structure of some sort. Is that significantly better than having a `releaseInstance` contract? Pure JSR-330 implementations have no @PreDestroy concept. Nor does Guice AFAIK. Today a Bean Validation implementation can cache ConstraintValidator and discard them upon memory pressure or if they are never used. If the @PreDestroy pattern is used, then this optimization is useless as the object would not be garbage collected until after the CVF is explicitly destroyed. It is possible to pass a ConstraintValidatorFactory instance for a specific validator instance Validator validator = validatorFactory .usingContext() .constraintValidatorFactory(someCVFInstance) .getValidator(); In this case and according to the current draft of BV 1.1, the Bean Validation provider will likely call `getInstance` and `releaseInstance` for each validate* calls on the `Validator`. It could technically keep the custom CVF instance around to check if it is reused in a subsequent `Validator` creation. Because of: - the custom CVF instance per Validator - the ability to offload cached ConstraintValidator instances - the ability to implement a dump BV provider that caches nothing I added what turned out to be a mysterious provision in the spec So from what I understand, the choice is between: - explicit releaseInstance and VF.close() with the benefit of more optimizations wrt memory usage - at least not involving custom weak reference like implementations - rely on the @PreDestroy approach - thus limiting CVF implementations with such feature - bringing simplicity to the BV provider implementations and removing VF.close() Thoughts? Emmanuel On Mon 2012-12-03 12:44, Gunnar Morling wrote: