Good way to return on locked mutex in go

Viewed 17256

Following problem: I have a function that only should allow one caller to execute. If someone tries to call the function and it is already busy the second caller should immediatly return with an error.

I tried the following:

1. Use a mutex

Would be pretty easy. But the problem is, you cannot check if a mutex is locked. You can only block on it. Therefore it does not work

2. Wait on a channel

var canExec = make(chan bool, 1)

func init() {
    canExec <- true
}

func onlyOne() error {
    select {
    case <-canExec:
    default:
        return errors.New("already busy")
    }

    defer func() {
        fmt.Println("done")
        canExec <- true
    }()

    // do stuff

}

What I don't like here:

  • looks really messi
  • if easy to mistakenly block on the channel / mistakenly write to the channel

3. Mixture of mutex and shared state

var open = true

var myMutex *sync.Mutex

func canExec() bool {
    myMutex.Lock()
    defer myMutex.Unlock()

    if open {
        open = false
        return true
    }

    return false
}

func endExec() {
    myMutex.Lock()
    defer myMutex.Unlock()

    open = true
}

func onlyOne() error {
    if !canExec() {
        return errors.New("busy")
    }
    defer endExec()

    // do stuff

    return nil
}

I don't like this either. Using a shard variable with mutex is not that nice.

Any other idea?

8 Answers

How about this package: https://github.com/viney-shih/go-lock . It use channel and semaphore (golang.org/x/sync/semaphore) to solve your problem.

go-lock implements TryLock, TryLockWithTimeout and TryLockWithContext functions in addition to Lock and Unlock. It provides flexibility to control the resources.

Examples:

package main

import (
    "fmt"
    "time"
    "context"

    lock "github.com/viney-shih/go-lock"
)

func main() {
    casMut := lock.NewCASMutex()

    casMut.Lock()
    defer casMut.Unlock()

    // TryLock without blocking
    fmt.Println("Return", casMut.TryLock()) // Return false

    // TryLockWithTimeout without blocking
    fmt.Println("Return", casMut.TryLockWithTimeout(50*time.Millisecond)) // Return false

    // TryLockWithContext without blocking
    ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
    defer cancel()

    fmt.Println("Return", casMut.TryLockWithContext(ctx)) // Return false


    // Output:
    // Return false
    // Return false
    // Return false
}

Lets keep it simple:

    package main
    
    import (
     "fmt"
     "time"
     "golang.org/x/sync/semaphore"
    )

    var sem *semaphore.NewWeighted(1)

    func init() {
      sem = emaphore.NewWeighted(1)
    }

    func doSomething() {
      if !sem.TryAcquire(1) {
        return errors.New("I'm busy")
      }
      defer sem.Release(1)
      fmt.Println("I'm doing my work right now, then I'll take a nap")
      time.Sleep(10)
    }

    func main() {
      go func() {
        doSomething()
      }()
    }

But the problem is, you cannot check if a mutex is locked. You can only block on it. Therefore it does not work

With possible Go 1.18 (Q1 2022), you will be able to test if a mutex is locked... without blocking on it.

See (as mentioned by Go 101) the issue 45435 from Tye McQueen :

sync: add Mutex.TryLock

This is followed by CL 319769, with the caveat:

Use of these functions is almost (but not) always a bad idea.

Very rarely they are necessary, and third-party implementations (using a mutex and an atomic word, say) cannot integrate as well with the race detector as implementations in package sync itself.

The objections (since retracted) were:

Locks are for protecting invariants.
If the lock is held by someone else, there is nothing you can say about the invariant.

TryLock encourages imprecise thinking about locks; it encourages making assumptions about the invariants that may or may not be true.
That ends up being its own source of races.

Thinking more about this, there is one important benefit to building TryLock into Mutex, compared to a wrapper:
failed TryLock calls wouldn't create spurious happens-before edges to confuse the race detector.

And:

A channel-based implementation is possible, but performs poorly in comparison.
There's a reason we have sync.Mutex rather than just using channel for locking.

Related