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

Mutex#unlock with block #10663

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

didactic-drunk
Copy link
Contributor

No description provided.

@asterite
Copy link
Member

Hi! Can you provide a use case for this?

@didactic-drunk
Copy link
Contributor Author

didactic-drunk commented Apr 28, 2021

For the same reason as Spinlock#unsync?

It's hard to unlock small section of code when using blocks, loops, or user provided blocks especially when they may throw an exception leaving a data structure unprotected.

lock.synchronize do
  ...locked section manipulation...
  optional_loop_that_needs_to_be_locked.each do
    optional_method_that_needs_to_locked` do
      val = lock.unlock { yield }
      optional_do_something_locked_with(val)
    end
  ...locked section manipulation...  
end

@straight-shoota
Copy link
Member

Sounds good to me.

I'm not sure about the name, though. Can we find a better one? Its behaviour is different from the existing #unlock.

@asterite
Copy link
Member

Just noting that this doesn't exist in Ruby, so I do wonder whether this is needed or that common. You can always do:

m.unlock
begin
   # ...
ensure
  m.lock
end

right?

@didactic-drunk
Copy link
Contributor Author

Sounds good to me.

I'm not sure about the name, though. Can we find a better one? Its behaviour is different from the existing #unlock.

Could use #unsync like Spinlock.

unlock was chosen attempting to be more crystal like and ignore rubyisms. Many methods take an optional block
such as File#open. It seems natural for #lock or #unlock with a block to change the state within the block
and put it back on exit.

On the same topic renaming #synchronize -> #lock seems more coherent with crystal naming and functions. Ruby doesn't have multi methods with and without a block. I assume that's why they chose to use #synchronize.

@didactic-drunk
Copy link
Contributor Author

Just noting that this doesn't exist in Ruby, so I do wonder whether this is needed or that common. You can always do:

m.unlock
begin
   # ...
ensure
  m.lock
end

right?

That's the code I used at first. When I needed it a 2nd time in the same class I made a method. There's an identical (now unused) method in Spinlock so I assumed it may get some use in crystal too.

Ultimately this issue isn't important. If you think the use is too rare for inclusion please close the PR. No hard feelings.

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

Successfully merging this pull request may close these issues.

None yet

3 participants