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

process based mutex #56

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

Conversation

julien-leclercq
Copy link
Contributor

Hello,

Here is a first draft for a process based Mutex implementation.

The idea is that when you call lock on a Mutex

  • it will send a Lock message to the Mutex implementation
  • the current process will wait to receive a LockAck message wrapping the current Mutex value
  • if the mutex is currently locked it will enqueue the caller pid in its internal state
  • when the Mutex process gets unlocked (or as soon as it receives a lock query if it's unlocked);
    • it will elect the next locker pid (first in the queue)
    • monitor the locker process
    • go in a locked state until the locker process dies or the caller process sends an Unlock message
  • when the locker process does not need the lock anymore, it sends an Unlock message containing the new value for the mutex.

I would like to add tests but what I have locally is a bit clumsy, so I thought I would PR this for comment already.

I would specially like to drag your attention on what should be done in case of a dead Mutex process, I am not sure how I could assert that the mutex is still alive before trying to lock on it.

Comment on lines +69 to +79
(* PR-Note (from: @julien-leclercq): How to handle the case of a dead mutex process ?*)
let lock mutex =
let () = send mutex @@ Server.Lock (self ()) in
let rec do_receive () =
match receive () with
| Server.(LockAck (sender, value)) when sender = mutex -> value
| msg ->
send (self ()) msg;
do_receive ()
in
do_receive ()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

☝️ here is the part where we could be in trouble if the Mutex process is dead

@leostera leostera force-pushed the main branch 2 times, most recently from 7179093 to b1364dd Compare February 7, 2024 21:58
@leostera
Copy link
Collaborator

leostera commented Mar 1, 2024

Hi @julien-leclercq! Thanks for the PR 🙏🏼

I think I'd like us to explore an API that is not functor-based, so its a little easier to use.

For example, if we had Mutex.start_link value function that returns a 'value Mutex.t we should be able to just create new mutexes for any type:

let m_int : int Mutex.t = Mutex.start_link 2112 in
let m_band : band Mutex.t = Mutex.start_link { name = "rush" } in

Another thing is that I'd love to understand if we want to treat a mutex as a "box" that we can swap the contents of or a "box" that we can access the contents of. I think the box approach is very nice, but maybe we want an API that only gives us access to the resource within the mutex while we are holding the mutex.

let m_band = Mutex.start_link { name = "rush" } in
Mutex.lock m_band (fun band ->
  (* do something with band *)
);

What do you think?

@leostera leostera added this to the riot/phase-2 milestone Mar 2, 2024
@leostera leostera removed this from the riot/phase-2 milestone Apr 11, 2024
@FayCarsons
Copy link

FayCarsons commented Apr 16, 2024

Hi! I volunteered to do #46 but I think I'm blocked on this as I need a Riot-friendly Mutex. I noticed it's been a little while since you've done any work on this, is it alright if I pick this up? @julien-leclercq

Edit: oop, meant to put this in the issue 😵‍💫

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 this pull request may close these issues.

3 participants