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 #726

Closed
tsofitvail20 opened this issue Jul 19, 2020 · 1 comment · Fixed by #729
Closed

Thread Safety #726

tsofitvail20 opened this issue Jul 19, 2020 · 1 comment · Fixed by #729

Comments

@tsofitvail20
Copy link

tsofitvail20 commented Jul 19, 2020

Hi,

We've monkeypatched about thread safety as follows:

        def klass_name
          return @klass_name if defined?(@klass_name)
          @klass_name = name.scan(/(.*)Session/)[0]
          @klass_name = klass_name ? klass_name[0] : nil
        end

https://github.com/binarylogic/authlogic/blob/master/lib/authlogic/session/base.rb#L784

Expected Behavior

The @klass_name will return only after we will extract the klass_name from the array (klass_name[0])

Actual Behavior

When there are multiple calls to the method simultaneously in a multi-threaded context, it happens occasionally that the line:
@klass_name = name.scan(/(.*)Session/)[0]
is executed, and then the other thread reaches the line:
return @klass_name if defined?(@klass_name)
which will return the @klass_name as the intermediate array.

So for example, if name is "UserSession", name.scan(/(.*)Session/)[0] returns ["User"], which is the value received in the other thread.

Suggested solution:
Assign @klass_name in one row will ensure that @klass_name will get value only after we will take out the value from the array.

        def klass_name
          return @klass_name if defined?(@klass_name)
          @klass_name = name.scan(/(.*)Session/)[0]&.first
        end

I'm new to OSS so I'm not comfortable submitting a patch, but I wanted to open an issue so someone can fix this for everyone.

@jaredbeck
Copy link
Collaborator

Nice find!

I'm new to OSS so I'm not comfortable submitting a patch ..

Your analysis and solution looks good to me. You were 99% there. Try a PR next time. If you can correctly analyze a threading issue, you can do a PR. Check out "Creating a pull request".

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

Successfully merging a pull request may close this issue.

2 participants