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

AtomicBoolean: add compare_and_set API #540

Open
vemv opened this issue Jun 24, 2016 · 7 comments
Open

AtomicBoolean: add compare_and_set API #540

vemv opened this issue Jun 24, 2016 · 7 comments
Labels
enhancement Adding features, adding tests, improving documentation. looking-for-contributor We are looking for a contributor to help with this issue. medium-priority Should be done soon.

Comments

@vemv
Copy link

vemv commented Jun 24, 2016

Compare-and-set is a useful functionality (that's why it exists in the Java API). It can save one from race conditions, as explained below.

Use case

  • Assume an atomicboolean called LOCKED, set to false.
  • Multiple concurrent threads run the following code:
if !LOCKED.value
  LOCKED.value = true
  # do something ....
  LOCKED.value = false
end

In this scenario, concurrent threads can read a false in if !LOCKED.value, and enter the if block. That defeats the purpose of an atomicboolean-backed lock.

If there was a CAS API, the fixed code would look like this:

if LOCKED.compare_and_set(false, true)

Furthermore

Ruby has no "tryLock" either. So it's not immediately obvious how to create non-blocking synchronized code. Boolean CAS would offer a nice alternative which Java programmers (and others) already are familiar with.

What do you think?

Cheers - Victor

@vemv
Copy link
Author

vemv commented Jun 24, 2016

Correction: Ruby does have try_lock, but no try_lock do ... end which would be more useful.

@jdantonio
Copy link
Member

@vemv Thank you for sharing your thoughts. I think this is a great idea and I'd love more of your input.

There are inconsistencies in the APIs of our atomic classes. They originally came from different places. AtomicReference was merged in from the atomic gem whereas AtomicFixnum and AtomicBoolean were created for specific internal needs. Ultimately the APIs should be as consistent as possible.

We've recently begun discussing concurrent-ruby 2.0. One of our goals will be more consistent APIs. Would you be interested in helping define what the API for the atomics should look like? If so, please feel free to create an issue with a proposal as a starting point for that discussion. I know that's a lot for me to ask but it would be of great help to us if you are interested and have time.

@vemv
Copy link
Author

vemv commented Jun 24, 2016

Would you be interested in helping define what the API for the atomics should look like?

Personally I would be fine with most possible reasonable designs, as long as the operations are there.

Just note that at least in Java there's no AtomicDouble/AtomicFloat so implementing CAS for your AtomicFixnum might or might not fully make sense.

@jdantonio
Copy link
Member

Certain operations might not make sense on all atomic classes, but we should be consistent with as much as possible across all atomic classes. That will be our goal.

@pitr-ch
Copy link
Member

pitr-ch commented Jun 25, 2016

@vemv have a look at make_true, it's a CAS operation with different naming.

@vemv
Copy link
Author

vemv commented Jun 25, 2016

It might be CAS as an implementation detail, but for the user it's not CAS as it doesn't accept a comparison argument (see the Java API for compareAndSet)

@pitr-ch
Copy link
Member

pitr-ch commented Jun 26, 2016

Ah, you are proposing to add the missing compare_and_set, not looking for a way how to fix the example, sorry I've focused on the latter. It definitely makes sense to add the missing CAS method.
Noting that the current make_true and make_false are fully replacing it:

  • CAS false, true -> make_true
  • CAS true, false -> make_false

even though 2 different methods have be inconveniently called.

@jdantonio jdantonio added the design Placeholder issues for more general design discussions. label Jun 26, 2016
@jdantonio jdantonio added this to the 2.0.0 milestone Jun 26, 2016
@jdantonio jdantonio self-assigned this Jun 26, 2016
@pitr-ch pitr-ch removed this from the 2.0.0 milestone Jul 6, 2018
@pitr-ch pitr-ch added enhancement Adding features, adding tests, improving documentation. looking-for-contributor We are looking for a contributor to help with this issue. medium-priority Should be done soon. and removed design Placeholder issues for more general design discussions. labels Jul 6, 2018
@pitr-ch pitr-ch changed the title AtomicBoolean: there's no compare_and_set API AtomicBoolean: add compare_and_set API Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adding features, adding tests, improving documentation. looking-for-contributor We are looking for a contributor to help with this issue. medium-priority Should be done soon.
Development

No branches or pull requests

3 participants