proposal: sync: add Mutex.LockContext
nhooyr opened this issue · 8 comments
I propose we add the following function to the sync package:
// LockContext is like Lock but if the ctx is cancelled before
// the mutex is available, then it returns ctx.Error().
//
// It returns with nil if the mutex was successfully locked.
func (m *Mutex) LockContext(ctx context.Context) error {
// ...
}At the moment, you can mimic this your own mutex based on make(chan struct{}, 1).
Every lock would correspond to a select on the ctx done chan and a send on the mutex channel.
And an unlock would read the value out of the channel.
This works and is what I use in my websocket library but it might be more generally useful and deserve a spot in the standard library.
I've often found myself needing it when I want to acquire a resource but the resource may be acquired for somewhat long periods of time by another caller and so in order to keep my code responsive, I need any mutex Lock calls to respect the context.
Personally I think the channel based mutex is a better choice here. sync.Mutex is most effective for locks that are held for a short period of time. I don't see how to implement this feature without complicating and slowing down sync.Mutex, which seems like a bad tradeoff for most programs.
If that's the case, I agree it's not a good idea to add it into sync.Mutex.
Perhaps it'd be better as a new ContextMutex type? We could just use channels under the hood to avoid a complex implementation. The channel approach while works isn't very discoverable or semantic.
cc @bcmills who has thought about this a lot
A package that wraps up some of the channel-based approaches sounds useful, although it could live outside the standard library.
A package that wraps up some of the channel-based approaches sounds useful, although it could live outside the standard library.
Agreed.
What other patterns do you think could be in this package? Others that come to my mind require generics.
Based on the discussion above, this proposal - add Mutex.LockContext - seems like a likely decline.
(It would still be interesting to see a channel-based mutex helper package outside the standard library.)
Agreed. I've created https://github.com/nhooyr/ctxsync but haven't implemented it yet.
Will close this for now. Feel free to watch the repo for updates.
Retracted.