douqiao6015
douqiao6015
2019-08-29 00:53

如何两次解锁互斥锁

Is it safe to unlock a mutex twice? My code:

var m sync.RWMutex = sync.RWMutex{}

func Read() {
    m.RLock()
    defer m.RUnlock()

    // Do something that needs lock
    err := SomeFunction1()
    if err != nil {
        return
    }

    m.RUnlock()

    // Do something that does not need lock
    SomeFunction2()

}

I need defer m.RUnlock() for the case SomeFunction1() returns error. But when SomeFunction1() returns without error, m will be unlocked twice by m.RUnlock() and defer m.RUnlock().

Is it safe to unlock the mutex twice? If not, how should I fix my code?

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

3条回答

  • doulongti5932 doulongti5932 2年前

    Is it safe for unlocking a mutex twice?

    Nope, you shouldn't unlock the mutex twice. It's a run-time error according to docs.

    RUnlock undoes a single RLock call; it does not affect other simultaneous readers. It is a run-time error if rw is not locked for reading on entry to RUnlock.


    If not, how should I fix my code?

    I would recommend to keep the defer but only m.RUnlock() in case of error. This can easily scale in case you add more function calls between SomeFunction1() and SomeFunction2().

    func Read() {
        var err error
        m.RLock()
        defer func() {
            if err != nil {
                m.RUnlock()
            }
        }()
    
    
        // Do something that needs lock
        err = SomeFunction1()
        if err != nil {
            return
        }
    
        m.RUnlock()
    
        // Do something that does not need lock
        SomeFunction2()
    }
    

    Try it on Go Playground!

    点赞 评论 复制链接分享
  • dt20081409 中国通信 2年前

    Unlocking an unlocked Mutex will cause a panic.

    You can simply remove the defer and add it in the if condition:

    var m sync.RWMutex = sync.RWMutex{}
    
    func Read() {
        m.RLock()
    
        // Do something that needs lock
        err := SomeFunction1()
        if (err != nil) {
            m.RUnlock()
            return
        }
    
        m.RUnlock()
    
        // Do something that does not need lock
        SomeFunction2()
    }
    

    Or even better (with a minor impact on readability):

    func Read() {
        m.RLock()
    
        err := SomeFunction1()
        m.RUnlock()
        if (err != nil) {
            return
        }
    
        SomeFunction2()
    }
    
    点赞 评论 复制链接分享
  • dongmanni6916 dongmanni6916 2年前

    From godoc:

    It is a run-time error if m is not locked on entry to Unlock.

    So, don't do that.

    Easiest fix is to not use defer unlock, but unlock it at every possible exit. Or, you can also do this, but it is not easy to read:

    func Read() {
        if err := func() {
          m.RLock()
          defer m.RUnlock()
    
          // Do something that needs lock
          err := SomeFunction1()
          if err != nil {
              return err
          }(); err != nil {
            return err
         }
    
        // Do something that does not need lock
        SomeFunction2()
    
    }
    
    点赞 评论 复制链接分享

为你推荐