We're updating the issue view to help you get more done. 

HibernateConstraintValidators are not correctly cached

Description

Let's say we have a HibernateConstraintValidator:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 import javax.validation.ConstraintValidatorContext; import javax.validation.metadata.ConstraintDescriptor; import org.hibernate.validator.constraintvalidation.HibernateConstraintValidator; import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorInitializationContext; public class SomeHibernateConstraintValidator implements HibernateConstraintValidator<SomeHibernateConstraintValidatorConstraint, String> { Foo payload; @Override public void initialize(ConstraintDescriptor<SimpleHibernateConstraintValidatorConstraint> constraintDescriptor, HibernateConstraintValidatorInitializationContext initializationContext) { // Here I can access following members of the initializationContext and do something with them: initializationContext.getScriptEvaluatorForLanguage("someLanguageName"); initializationContext.getClockProvider(); initializationContext.getTemporalValidationTolerance(); // I even want to "save" the constraint validator payload so I can access it later payload = initializationContext.getConstraintValidatorPayload(Foo.class); } @Override public boolean isValid(String value, ConstraintValidatorContext context) { doSomething(this.payload); // ... } }

Alright.
Now we use that SomeHibernateConstraintValidator by creating different Validators with different (initialization)Contexts from the same ValidatorFactory:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 final HibernateValidatorFactory hvf = someValidatorFactory.unwrap(HibernateValidatorFactory.class); // Obtain a validator with 5 days temporal validation tolerance hvf.usingContext() .temporalValidationTolerance(Duration.ofDays(5)) .getValidator().validate(...); // Here a new SomeHibernateConstraintValidator instance gets created and // it's initialize(...) method gets called with a 5 days duration. // That SomeHibernateConstraintValidator instance also gets cached (by ConstraintValidatorManager) // Now let's obtain another validator with 10 days temporal validation tolerance hvf.usingContext() .temporalValidationTolerance(Duration.ofDays(10)) .getValidator().validate(...); // Now the just cached SomeHibernateConstraintValidator gets used... However I would expect that // a new SomeHibernateConstraintValidator instance gets created and and it's initialize(...) method gets called // with the 10 days duration! If you ask me that is a BUG!

You see the problem I described in the comments.
The same applies to the .scriptEvaluatorFactory(...) and .clockProvider(...) methods.

So the issue here is that the members of a HibernateConstraintValidatorInitializationContext instance are not taken into account when caching a HibernateConstraintValidator.
The solution is that these three members should be part of the caching key when caching a constraint validator.

For .constraintValidatorPayload(...) however (the fourth member of a HibernateConstraintValidatorInitializationContext) it's a different story.
First of all who knows what the payload object is and/or what it references? And how many different payload objects there will be during a validator factory lifetime (which usually is the app lifetime).
In our case (web application) we create a new, different payload object for EACH request - plus such a payload is only be needed for the corresponding request (lifetime).
So if we cache each HibernateConstraintValidator instance (in ConstraintValidatorManager) that would mean if we have thousands of requests per second, we put thousands of constraint validator instances in our cache - per second... And we don't even need them anymore!
Sounds like a memory leak.
Because the payload could be any object really but isn't a type/class provided by Hibernate Validator and therefore out of our control we don't know what it is, how big it is, what if references and how many there will be.
So I propose to never ever cache a constraint validator that could save a reference to such a payload. This condition applies if a) a payload is supplied in the current context (is !=null) AND b) only instances of HibernateConstraintValidatorInitializationContext - because only that classes get a payload passed via their initialize(...) method.

Environment

Doesn't matter.

Status

Assignee

Matthias Kurz

Reporter

Matthias Kurz

Labels

Worked in

None

Feedback Requested

None

Feedback Requested By

None

backPortable

None

Community Help Wanted

None

Suitable for new contributors

None

Requires Release Note

None

backportDecision

None

backportReEvaluate

None

Components

Fix versions

Affects versions

6.0.8.Final
6.0.5.Final
6.0.6.Final
6.0.7.Final

Priority

Major