Fix ConcurrentModificationException

[romm1of]

@murdos could you please approve the awaiting workflow?

[murdos]

@murdos could you please approve the awaiting workflow?

Sure. Done.

[romm1of]

@murdos it seems there is much more to the problem than just unsafe easy random initialization. What I've discovered is that there is a recursive initialization of easy random. As of now it works like this:

1. new instance of easy random is created
2. ProtobufRandomizer registry is created via SPI
3. Easy Random calls init for all existing registries
4. when nextObject is called for proto message -> getRandomizer is called for ProtobufRandomizer which instantiates new easy random
5. steps 1-3 are repeated

step 3 is failing simply because the same EasyRandomParameters is shared among many easy random instances and for example ExclusionRandomizerRegistry's init method populates Set<Predicate> fieldPredicates & Set<Predicate> typePredicates while at the same another thread can be calling getRandomizer on the same ExclusionRandomizerRegistry which iterates over the same fields which usually results in ConcurrentModificationException.

It's hard for me to say how exactly this issue should be addressed. I've got a few ideas:

1. completely remove EasyRandom from this lib (this would remove the recursive instantiation issue which would fix everything)
2. refactor EasyRandom's RandomizerRegistry.init() method signature to have EasyRandom as parameter instead of EasyRandomParameters . This would also remove the unnecessary instantiation
3. move EasyRandom's ExclusionRandomizerRegistry.init() logic to constructor, I guess this might fix the ConcurrentModificationException triggering in that registry, but this does not fix the recursive instantiation.

what do you think?

update: created new method in easy-random's RandomizerRegistry which sets the EasyRandom instance. The test is OK, no ConcurrentModificationException is thrown. Going to open a PR on easy-random's repository

[murdos]

@romm1of Please check my work in #170 : I've reworked message generation to not use anymore an embedded EasyRandom instance, but use RandomizerRegistry available through EasyRandomParameters (it was not reliable before, but will fixed in 6.0 see https://github.com/j-easy/easy-random/pull/482#discussion_r1259182509).

[romm1of]

@murdos good job. Not only did you fix the concurrency issue, but also introduced a caching so that same randomizers can be reused for the same types instead of creating new one. (for that I had to use a workaround of having an another caching registry) I've run the concurrency tests multiple tests, everything is OK. I'm going to close this PR and https://github.com/j-easy/easy-random/pull/502 as well.

I know that snapshot libs are not for production use, but guess I'll have no option but to use them anyway since prior to that I had a simple lock on every EasyRandom.next<>() invocation, that fixed a problem with concurrency but on huge CI builds where easy random is actively used it increased the build time dramatically.

p.s. when you've merged the changes could you please publish the snapshot artifact? Thanks

[romm1of]

fixed in https://github.com/murdos/easy-random-protobuf/pull/170

[murdos]

@romm1of : snapshot is now available with version 1.0.0-SNAPSHOT :-)