HibernateConstraintValidators are not correctly cached

Description

Let's say we have a HibernateConstraintValidator:

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

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

Feedback Requested

None

Feedback Requested By

None

backPortable

None

Suitable for new contributors

None

backportDecision

None

backportReEvaluate

None

Components

Fix versions

Affects versions

Priority

Major
Configure