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

Thread-safety issues on Rubinius #300

Closed
glebm opened this issue Sep 18, 2018 · 0 comments
Closed

Thread-safety issues on Rubinius #300

glebm opened this issue Sep 18, 2018 · 0 comments
Labels
Milestone

Comments

@glebm
Copy link
Owner

glebm commented Sep 18, 2018

Code like this:

@cache[key] || @mutex.synchronize { @cache[key] ||= fetch_value }

Is not thread-safe and is flaky (fails CI sometimes) on Rubinius.

Instances of this in the codebase are:

@cache[absolute_path] || @mutex.synchronize { @cache[absolute_path] ||= super }

def get(**file_finder_args)
@cache[file_finder_args] || @mutex.synchronize do
@cache[file_finder_args] ||= begin
args = file_finder_args.dup

This one should be OK, as the instance variable is set (to nil) in the constructor (so the ivars keys are not modified):

@cached_paths || @mutex.synchronize { @cached_paths ||= super }

@glebm glebm added the bug label Sep 18, 2018
@glebm glebm closed this as completed in 1bdae5f Sep 18, 2018
@glebm glebm added this to the v0.9.25 milestone Sep 18, 2018
glebm added a commit that referenced this issue Sep 18, 2018
Also, clear `@computation` after we obtained the value, as we no longer
need it.

Refs #300
glebm added a commit that referenced this issue Sep 18, 2018
Also, clear `@computation` after we obtained the value, as we no longer
need it.

Refs #300
glebm added a commit that referenced this issue Sep 18, 2018
glebm added a commit that referenced this issue Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant