FakerValues initialization isn't thread safe

[ttuckergw]

Describe the bug
We currently use JavaFaker with Gatling for load testing and would like to switch to DataFaker, but there is a multi-threaded initialization bug in JavaFaker FakeValues.get() that we would like to report. I downloaded the DataFaker sources for version 1.7.0, and I see it still has that same JavaFaker code. It is a pretty trivial bug. That method should be a synchronized Java method so that SnakeYaml isn’t invoked multiple times in a multi-threaded environment to initialize the same fake values data.

To Reproduce
Run a Gatling load test with 100+ closed model users (or a java multi-threaded test with 100+ threads) that uses a single Faker library (fake names for instance). You should see multiple invocations of SnakeYaml all trying to initialize the Fake Values for the same library simultaneously.

Expected behavior
The FakeValues code should only need to initialize the FakeValues for each library once in a multi-threaded environment.

Versions:
DataFaker 1.7.0 (and the current JavaFaker release 1.0.2)

Additional context
We see this issue with Gatling load testing.

[snuyanzin]

do you have a bit more details or code sample?

[ttuckergw]

Hi Sergey, I can create a test case from our code. Where do I upload it? From: Sergey Nuyanzin ***@***.***> Date: Wednesday, December 21, 2022 at 10:50 AM To: datafaker-net/datafaker ***@***.***> Cc: Tim Tucker ***@***.***>, Author ***@***.***> Subject: {EXT} Re: [datafaker-net/datafaker] FakerValues initialization isn't thread safe (Issue #574) do you have a bit more details or code sample? — Reply to this email directly, view it on GitHub<#574 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A44UANA6FQLN3QB34POCCYLWONGPDANCNFSM6AAAAAATF5PT4Q>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[snuyanzin]

here would be ok

[bodiam]

@ttuckergw could you try the latest snapshot version, and check if it's resolved there?

[ttuckergw]

No problem, I’ll download it this afternoon and give it a try. From: Erik Pragt ***@***.***> Date: Wednesday, December 21, 2022 at 1:40 PM To: datafaker-net/datafaker ***@***.***> Cc: Tim Tucker ***@***.***>, Mention ***@***.***> Subject: {EXT} Re: [datafaker-net/datafaker] FakerValues initialization isn't thread safe (Issue #574) @ttuckergw<https://github.com/ttuckergw> could you try the latest snapshot version, and check if it's resolved there? — Reply to this email directly, view it on GitHub<#574 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A44UANEPHF3MONCLAB676C3WON2L5ANCNFSM6AAAAAATF5PT4Q>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[ttuckergw]

Hi Erik, I downloaded the datafaker 1.8.0-SNAPSHOT library late yesterday and so far I’ve run two large Gatling tests that have both passed. The second one removed all of the locking we had added around the javafaker library calls as a work-around for the locking bug I filed. I think the fix is good. Thank you very much for fixing that! Tim From: Erik Pragt ***@***.***> Date: Wednesday, December 21, 2022 at 1:40 PM To: datafaker-net/datafaker ***@***.***> Cc: Tim Tucker ***@***.***>, Mention ***@***.***> Subject: {EXT} Re: [datafaker-net/datafaker] FakerValues initialization isn't thread safe (Issue #574) @ttuckergw<https://github.com/ttuckergw> could you try the latest snapshot version, and check if it's resolved there? — Reply to this email directly, view it on GitHub<#574 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A44UANEPHF3MONCLAB676C3WON2L5ANCNFSM6AAAAAATF5PT4Q>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[bodiam]

@ttuckergw don't thank me, but @snuyanzin. He is the concurrency expert here, and fixed the issue!

Thanks for confirming that this fixed the issue!

[valfirst]

is there any plan to release this fix soon? we've also faced this issue, snapshots are good for testing, but it'll be safer to use stable release version for production

[bodiam]

@valfirst No, there's no plan yet for a new release at this moment, but our snapshots are pretty stable. I was thinking of creating a new release every 100 stars or so as an experiment, so.... 99 stars to go :-)