Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect use of double checking singleton idiom #6

Open
mkrussel77 opened this issue Mar 15, 2018 · 1 comment
Open

Incorrect use of double checking singleton idiom #6

mkrussel77 opened this issue Mar 15, 2018 · 1 comment

Comments

@mkrussel77
Copy link

The current Faker.with(Context) is broken if used by multiple threads. The mFaker needs to be declared as volatile, or someone may get an partially initialized instance of it. Also if anyone accesses the static fields like Lorem, they may get incomplete instances if they do not synchronize of Faker.class first.

https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java

As a whole nothing seems to be trying to be thread safe in this library other than Faker.with(), which does so incorrectly. Probably need to look into each class and see if they can be made thread safe. The methods that act on Views don't need to be, but the generic data generation methods should be thread safe.

An example. LoremComponent, would be thread safe if loremWords was declared final, since the final will make sure anyone that gets an instance of it will see the list fully constructed. And if FakerCoreComponent had the context declared as final.

@thiagokimo
Copy link
Owner

Thanks for the feedback @mkrussel77!
I'll clean-up the code and write some tests in order to verify the problem.

In addition, you are very welcome to make a pull request in case you already have the fixes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants