Hacker News new | ask | show | jobs
by Xeoncross 316 days ago
> and 'f' panics, the program's deadlocked forever

I don't see `mu.Lock(); f(); mu.Unlock()` anywhere really.

`mu.Lock(); defer mu.Unlock(); f();` is how everyone does it to prevent that possibility.

4 comments

From the golang github org in non-toy code:

1. https://github.com/golang/tools/blob/f7d99c1a286d6ec8bd4516a...

2. https://github.com/golang/sync/blob/7fad2c9213e0821bd78435a9...

There are dozens and dozens throughout the stdlib and other popular go code.

The singleflight case is quite common, if you have:

    mu.Lock()
    if something() {
      mu.Unlock()
      moreWorkThatDoesntWantMutex()
      return
    }
    mu.Unlock()
most gophers use manual lock/unlocks to be able to unlock early in an 'if' before a 'return', and that comes up often enough that it really does happen.

I see manual lock/unlock all the time, and semi-regularly run into deadlocks caused by it. Maybe you don't use any third-party open source libraries, in which case, good for you congrats.

Maybe it's stockholm syndrome, but the "manual" unlocks in bite sized funcs don't bother me -- it's easy to see that it's going to be unlocked by the time the function returns.
Unless there’s a bug and it panics?

At a minimum now you’ve also got to scan for nil dereferences. Maybe that simple code between the mutex is pulled out into a function in a later commit. Then the function body is changed in the next commit to do something less simple.

This is all depressingly reminiscent of listening to die-hard C enthusiasts insist that avoiding use-after-free bugs and out of bounds reads and other issues are easy as long as you write code perfectly all the time. It’s easy as long as you just always do the right thing.

It’s okay to admit your language of choice has weaknesses. You don’t have to stop using go just because you can acknowledge that data races can creep in if you’re not diligent enough.

I'm confused. I was responding to a complaint about "manual unlocks" (vs. the "certainty" of a Lock(); defer Unlock().

The examples pointed to in the stdlib to complain about that usage pattern could have easily done just that -- but they were optimized to shrink the window of the locking time. My take is that the code paths were simple enough to verify that it was safe to do, and being in the stdlib had both. enough eyes to validate and a driver (being foundational code).

I recognize that Go is far from perfect but think some of the pearl clutching is manufactured (OMG! nil pointers! -- they technically can happen but in my many years of coding in Go it hasn't been a problem of note.)

And of course we should recognize that the locking issue being brought up was a simple one and the story gets worse when talking about complicated concurrency. Then the Go story of "just use the race detector!" is tempered by the fact that it is only best effort and no guarantees that it's found everything (like a bloom filter of certainty).

So year, Go has footguns and other inadequacies (FFI is a bummer). But it's good enough for a lot of things. Rust is on my list to learn, so my only complaint about it is the steep learning curve and I'm an old lazy dev.

Until you have to call a slow function after the mutex access leading to the lock being held long enough to cause problems.

Now you either refactor into multiple functions, while ensuring all copies of possibly shared data when passing function arguments are correctly guarded or ”manually” unlock when you don’t need the mutex access anymore.

OK, but you're not in "Go"-specific problems any more, that's just concurrency issues. There isn't any approach to concurrency that will rigorously prevent programmers from writing code that doesn't progress sufficiently, not even going to the extremes of Erlang or Haskell. Even when there are no locks qua locks to be seen in the system at all I've written code that starved the system for resources by doing things like trying to route too much stuff through one Erlang process.
I would say it is a Go specific problem with how mutexes and defer are used together.

In rust you would just throw a block around the mutex access changing the scoping and ensuring it is dropped before the slow function is called.

Call it a minimally intrusive manual unlock.

In Rust you can also explicitly drop the guard.

    drop(foo); // Now foo doesn't exist, it was dropped, thus unlocking anything which was kept locked while foo exists
If you feel that the name drop isn't helpful you can write your own function which consumes the guard, it needn't actually "do" anything with it - the whole point is that we moved the guard into this function, so, if the function doesn't return it or store it somewhere it's gone. This is why Destructive Move is the correct semantic and C++ "move" was a mistake.
You can also just drop it by scoping the mutex guard to the critical area using a block, since it’ll be dropped when it goes out of scope.
Generally, in any language, I'd suggest of you're fiddling with lots of locks (be they mutexes, or whatever), then one is taking the wrong approach.

Specifically for Go, I'd try to address the problem in CSP style, so as to avoid explicit locks unless absolutely necessary.

Now for the case you mention, one can actually achieve the same in Go, it just takes a bit of prior work to set up the infra.

  type Foo struct {sync.Mutex; s string}
  
  func doLocked[T sync.Locker](data T, fn func(data T)) {
      data.Lock(); defer data.Unlock(); fn(data)
  }
  
  func main() {
      foo := &Foo{s: "Hello"}
      doLocked(foo, func(foo *Foo) {
        /* ... */
      })
      /* do the slow stuff */
  }
> OK, but you're not in "Go"-specific problems any more, that's just concurrency issues.

It’s absolutely a go-specific problem from defer being function scoped. Which could be ignored if Unlock was idempotent but it’s not.

It's tedious, I agree, but I found it easiest to just wrap it in an inline function defined and called there and then.

This alleviates all these problems of unlocks within if bodies at the cost of an indent (and maybe slight performance penalty).

Amazing! You’ve found the mythical perfect programmers who never make mistakes and always run resource cleanup under defer!

I personally haven’t worked with any of these perfect programmers, but I’m glad that you do.

Trick question: what is the scope of `defer` in go?
Function body, so no don’t put it in your loop. Just break it out to a helper fn if needed. This isn’t a big problem in practice.
Yes, though I think tooling could be better; if I had more spare time I'd write a linter which flagged defers in loops that didn't come with an accompanying comment.
I always think, Go is open source why not just fork it to add feature XYZ, then I realize I am better off using languages whose communities appreciate modern language design, instead of wasting my spare time with such things.