Add withLock keyword for locking/unlocking mutexes around code

[d-torrance]

As suggested by @jkyang92 in #4299 (comment). This way, we have a way to not worry about getting stuck with a locked mutex if something happens before we have a chance to unlock it.

i1 : m = new Mutex

o1 = m

o1 : Mutex

i2 : last trap withLock(m, (print last trap tryLock m; 1/0))
pthread_mutex_trylock: Device or resource busy

o2 = division by zero

o2 : Error

i3 : tryLock m

i4 : unlock m

i5 : withLock(m, sleep 60)
  C-c C-cstdio:5:0:(3):[1]: error: interrupted

i6 : tryLock m

i7 : unlock m

AI Disclosure

Claude Code wrote the first draft of the documentation for withLock (but I ended up rewriting most of it...)

[mahrud]

I don't like this syntax. I also don't think Jay's suggestion involved the user specifying the mutex.

My impression after we talked was that there are two important cases:

• functions which should not be run by different threads simultaneously, in which case perhaps a mutex tucked inside the FunctionClosure would be great. If we go with this option, I would prefer going back to the "threadLock" syntax or similar.
• functions which should not run on the same input by different threads. This is more subtle, but I would prefer a syntax which doesn't involve the user ever creating or holding a mutex object directly (e.g. with new Mutex).

Based on my own experience trying to scale parallel computations, here is my current best proposal for the latter: make ht#k ??= (...) lock a mutex specifically for the key k in ht, so that the second thread has to wait until the first is finished and use its result.

The main downside I can think of is that you're forced to store the output in the cache, which can get annoying on very large thread counts (e.g. trying to compute and cache ht#k for k=1..100 on 100 different threads will be slow once all threads need get in line to lock ht to write their result.

One alternative is introducing a similar syntax that just locks but doesn't store, so maybe something like this:

ht#k   = (...) -- always compute and store
ht#k ??= (...) -- compute if not cached and store
ht#k ?:= (...) -- compute if not locked, but the := implies local, so nothing is stored

Here is a real-life example, since basis is not cached for modules:

basis(ZZ, Module) := Matrix => (d,M) -> M.cache#(basis, d) ?:= (...)

Once computation starts, a mutex is placed in M.cache#(basis, d) and locked; once computation is finished or errored, the mutex is unlocked and discarded (detail: if error lands you in the debugger, the mutex should remain locked), and nothing is cached.

I see a potential critique that not using a word like "lock" anywhere here might make the syntax hard to guess, but I think we could resolve this with a convention of where mutexes should be stored, e.g:

basis(ZZ, Module) := Matrix => (d,M) -> M.mutex#(basis, d) ?:= (...)

where M.mutex is a MutexTable which inherits from a CacheTable but perhaps with different optimizations to prevent the 100-thread issue above.

[d-torrance]

The goal here is to have something very general that can wrap arbitrary code like all the modifications of global variables in #4299. The syntax I think is essentially what Jay suggested, except withLock(lock, code) instead of the Haskell/Lean-style withLock lock code that we can't really do in Macaulay2.

[mahrud]

1. I believe in Jay's suggestion withLock automatically produces a mutex and assigns it to the label makeMonomialOrderingMutex without requiring the user to write makeMonomialOrderingMutex = new Mutex. This is a big difference IMO.
2. I agree that withLock lock code and withLock(lock, code) convey the same information, but they are not the same syntax. I'm fine with the former in Haskell/Lean, but dislike the latter in M2. For instance, note the double indentation cause by two parentheses here:

withLock(mutex, (
	...
	))

Also, I've already been using your threadLock keyword and don't understand the rationale for forcing the user to think about mutexes when they don't need to.

[d-torrance]

One issue with threadLock (I don't have a strong preference between that name or this name, btw) is that there was a global mutex that was always used, rather than a mutex local to that specific code. So I think providing a mutex is nice.

Maybe we could make it more flexible?

• One argument version -- use a global mutex so the user doesn't have to worry about it
• Two argument version:
  • First argument is a mutex - use that
  • First argument is a symbol - create a mutex and assign to that symbol

[d-torrance]

Also, regarding the parentheses -- I think the only way withLock lock code would work in Macaulay2 is if withLock lock returned a function. But it would have to be a function that can accept a Code rather than an Expr, which I think is only possible right now for keywords.

[mahrud]

My point was that we can't (and perhaps shouldn't) force the syntax of another language into Macaulay2 if the grammar isn't the same. It's like writing Farsi with English characters; Farsi idioms are beautiful but don't look good in Finglish XD

Regarding global mutexes, I think the best solution is to put it in the FunctionClosure, which is not global and can easily be garbage collected if necessary.

[d-torrance]

My point was that we can't (and perhaps shouldn't) force the syntax of another language into Macaulay2 if the grammar isn't the same. It's like writing Farsi with English characters; Farsi idioms are beautiful but don't look good in Finglish XD

👍

Regarding global mutexes, I think the best solution is to put it in the FunctionClosure, which is not global and can easily be garbage collected if necessary.

Which FunctionClosure? All we have at this point is a Code object -- it could be anything, and I think we want to lock the mutex before we evaluate it.

[jkyang92]

I think the suggestion that @mahrud has about putting mutexes in the function closure is somewhat orthogonal to the problem I think this tool solves.

@mahrud The version I'm proposing does not automatically produce a mutex. I view this as a simpler wrapper to around mutexes to make them safe against exceptions and early returns, both of which are a significant cause of mutexes never getting unlocked. The intent is to produce something that when a user does really need to use a mutex, the work is easier. I agree overall that I would prefer that most users never touch a mutex and use higher level constructs to ensure thread safety.

The semantics that @d-torrance is proposing here are essentially what I want. Again, I still view this as a lower level tool, just a safer one.

The specific syntax is not of the highest importance to me but I don't particularly like the current style if only because it's not clear that the code is executed with the lock held (because it's different from most other functions). A notation like withLock lock do code where withLock is a keyword like for would be good, but if we don't want to add a whole new code type then maybe if the syntax withLock_lock code worked, it would be fine.

PS:
A hack/trick to implement the part of @mahrud's suggestion where you want a function closure to have a mutex is to do something like the following:

withStaticMutex = (f) -> (
    mutex := new Mutex;
    f (mutex)
)

someMethod(ZZ,ZZ) := withStaticMutex ((mutex) ->
    (a, b) -> ( body of the function potentially referencing mutex ))

This exploits the ability of function closures to capture frames. the name withStaticMutex is a reference to static local variables in c++ and is a bit of a terrible name. Also, this doesn't do any of the automatic locking one might want, it's just a way to associate the data to a function using current code.

[d-torrance]

Using keywords with _ right now is a syntax error:

i1 : elapsedTime_foo bar
stdio:1:11:(3):[1]: error: syntax error at '_'

If we're going to add a new code type, maybe a more general Python-like context manager syntax would be nice? We could define with x do y to do this lock/unlock thing when x is a Mutex. Maybe if x is a file, we could call close at the end? And allow users to install methods for arbitrary types?

[d-torrance]

Closing in favor of #4367