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

Behavior of acquire/2 when changing max #6

Open
TylerPachal opened this issue Apr 19, 2021 · 1 comment
Open

Behavior of acquire/2 when changing max #6

TylerPachal opened this issue Apr 19, 2021 · 1 comment

Comments

@TylerPachal
Copy link

👋 Hello!

I am trying to understand what the intended behavior is supposed to be when acquiring a semaphore and changing the max at the same time to a lower number. Here is an example:

# First acquire should work
iex(1)> Semaphore.acquire(:a, 5)
true

iex(2)> Semaphore.count(:a)     
1

# Increasing the max and acquire again
iex(3)> Semaphore.acquire(:a, 7) 
true

iex(4)> Semaphore.count(:a)     
2

# Acquire with the same max
iex(5)> Semaphore.acquire(:a, 7)
true

iex(6)> Semaphore.count(:a)     
3

# Now acquire with a lower max, shouldn't this return false?
iex(7)> Semaphore.acquire(:a, 2)
true

# This count feels a little disingenuous because of the previous statement returning true
iex(8)> Semaphore.count(:a)     
2

What is the behavior supposed to be when lowering the max?

At first my intuition was that the count would be reset to 0, and that's why the iex(7) succeeded, but then when I look at the count I see 2, which is quite confusing. The 2 would make sense if the return value of iex(7) was false (i.e. the semaphore is still full but we now have a lower limit). Another possibility would be that changing the max via the acquire does not alter the count.

Additionally, I am not sure what the use case is for adjusting the max while acquiring at the same time. Would it makes things more clear to have a separate set_max function?

@TylerPachal TylerPachal changed the title Acquire behavior when changing counts Behavior of acquire/2 when changing max Apr 19, 2021
@TylerPachal
Copy link
Author

TylerPachal commented Apr 27, 2021

I have provided a potential fix for this issue: #7

Executing the same example as above now returns false for iex(7):

iex(1)> Semaphore.acquire(:a, 5)
true

iex(2)> Semaphore.count(:a)     
1

iex(3)> Semaphore.acquire(:a, 7) 
true

iex(4)> Semaphore.count(:a)     
2

iex(5)> Semaphore.acquire(:a, 7)
true

iex(6)> Semaphore.count(:a)     
3

iex(7)> Semaphore.acquire(:a, 2)
false # I return false now!

iex(8)> Semaphore.count(:a)     
2

It would be nice if the count returned on iex(8) was 3, but with the current implementation using the ETS counter functions I don't think it is possible.

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

No branches or pull requests

1 participant