[Validator][RecursiveContextualValidator] Prevent validated hash collisions

[fancyweb]

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	-
Deprecations?	-
Tickets	-
License	MIT
Doc PR	-

RecursiveContextualValidator uses spl_object_hash at different places to generate cache keys. The problem with this method is that there can be collisions (same hash for two different objects) since once an object is destroyed, spl_object_hash can return a previously used hash.

I encountered the problem on a project with constraints that are created on the fly. The first constraint is applied and is "cached". Then it is destroyed. Then another constraint happen to have the exact same spl_object_hash. It is skipped while it would cause violations. I was able to reproduce the problem consistently in the unit test.

The easy way to deal with this is to keep references to all objects (proposed solution). But I guess it could leak too much.

We could also try to "clean" the ExecutionContext stores at the end of the three validate methods. However, we would still need to keep references to the initialized objects since they must really be initialized one time, even on multiple calls

[HeahDude]

Why is this needed for group sequences?

[fancyweb]

GroupSequences can suffer from the same problem since they can be passed directly to the validate method as objects.

[nicolas-grekas]

We should find a place where these arrays can be emptied. The memory leak should be avoided.

[fancyweb]

We should find a place where these arrays can be emptied. The memory leak should be avoided.

I think we can clear constraints and group sequences arrays at the end of each validate methods. However, we can never clean the initialized objects. Would that work for you?

[fabpot]

@fancyweb What's the status of this PR?

[fancyweb]

I need to check Nicolas's suggestion, please keep it open.

[fancyweb]

We should find a place where these arrays can be emptied. The memory leak should be avoided.

I think we can clear constraints and group sequences arrays at the end of each validate methods. However, we can never clean the initialized objects. Would that work for you?

Unfortunately that does not work. I had to remove this part.

[xabbuh]

Can you update the FormValidator class to remove the $fieldFormConstraint = new Form() statements (can be replaced with $formConstraint). These statements were only added in #36865 to precisely prevent the collisions being fixed here.

[fancyweb]

@xabbuh I looked at this a few days ago but it needs more work. My patch does not fix all cases because it does not take into account the current defaultPropertyPath. One of FormValidator tests fails because of this. You can have a look if you want to.

[xabbuh]

continued in #38387, thanks for starting the work on this @fancyweb