Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Go’s race detector has a mutex blind spot

doublefree.dev

Post reply on HN
Print view View original

21–30 of 60 posts

Re: Go’s race detector has a mutex blind spot

#21

Post by jason_oster »

ViewTrick1002 wrote: post #7
klabb3 wrote: post #4

Earlier quoted context omitted.

> Rewrite your code in rust, and get something better than the go race detector every time you compile. Congrats, rustc forced you to wrap all your types in Arc >, and you no longer have data races. As a gift, you will get logical race conditions instead, that are even more difficult to detect, while being equally difficult to reproduce reliably in unit tests and patch. Don’t get me wrong, Rust has done a ton for saf…

A well formed Go program would have the same logical race conditions to manage as well. The Arc is only needed when you truly need to mutably share data. Rust like Go has the full suite of different channels and what other patterns to share data.

Small correction: The Arc is for sharing across threads, the Mutex is for mutation. But you are generally correct that they can be used independently.

Re: Go’s race detector has a mutex blind spot

#22

Post by TheDong »

Xeoncross wrote: post #11
TheDong wrote: post #10

Earlier quoted context omitted.

As a sibling said, Go has all the same deadlocks, livelocks, etc you point out that rust doesn't cover, in addition to also having data-races that rust would prevent. But, also, Go has way worse semantics around various things, like mutexes, making it much more likely deadlocks happen. Like in go, you see all sorts of "mu.Lock(); f(); mu.Unlock()" type code, where if it's called inside an `http.Handler` and 'f' panic…

> 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.

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.

Re: Go’s race detector has a mutex blind spot

#23

Post by ViewTrick1002 »

jerf wrote: post #20
ViewTrick1002 wrote: post #17

Earlier quoted context omitted.

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 li…

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.

Re: Go’s race detector has a mutex blind spot

#24

Post by johncolanduoni »

mr90210 wrote: post #8
klabb3 wrote: post #4

Earlier quoted context omitted.

> Rewrite your code in rust, and get something better than the go race detector every time you compile. Congrats, rustc forced you to wrap all your types in Arc >, and you no longer have data races. As a gift, you will get logical race conditions instead, that are even more difficult to detect, while being equally difficult to reproduce reliably in unit tests and patch. Don’t get me wrong, Rust has done a ton for saf…

> Congrats, rustc forced you to wrap all your types in Arc > Also, don’t people know that a Mutex implies lower throughput depending on how long said Mutex is held? Lock-free data structures/algorithms are attempt to address the drawbacks of Mutexes. https://en.wikipedia.org/wiki/Lock_(computer_science)#Disadv...

Lock-free and even wait-free approaches are not a panacea. Memory contention is fundamentally expensive with today’s CPU architectures (they lock, even if you ostensibly don’t). High contention lock-free structures routinely perform worse than serialized locking.

Re: Go’s race detector has a mutex blind spot

#25

Post by reactordev » 

    if id == 1 {
        counter++;
    }
Found your problem. /s

In all honesty, if you “do work” using channels then all your goroutines are “thread safe” as the channel keeps things in order. Also, mutex is working as intended. As you see in your post, -race sees this, it’s good. Now have one goroutine read from a chan, get rid of the mutex, all other goroutines write to the chan, perfection.

Re: Go’s race detector has a mutex blind spot

#26

Post by ViewTrick1002 »

jason_oster wrote: post #21
ViewTrick1002 wrote: post #7

Earlier quoted context omitted.

A well formed Go program would have the same logical race conditions to manage as well. The Arc is only needed when you truly need to mutably share data. Rust like Go has the full suite of different channels and what other patterns to share data.

Small correction: The Arc is for sharing across threads, the Mutex is for mutation. But you are generally correct that they can be used independently.

Of course. But if you’re using a channel then it hides the inner constructs.

Comparing writing a web service in Go and rust you would likely also utilize Tokio which has a wide variety of well designed sync primitives.

https://docs.rs/tokio/latest/tokio/sync/index.html

Re: Go’s race detector has a mutex blind spot

#27

Post by masklinn »

jerf wrote: post #20
ViewTrick1002 wrote: post #17

Earlier quoted context omitted.

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 li…

> 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.

Re: Go’s race detector has a mutex blind spot

#28

Post by gf000 »

Xeoncross wrote: post #11
TheDong wrote: post #10

Earlier quoted context omitted.

As a sibling said, Go has all the same deadlocks, livelocks, etc you point out that rust doesn't cover, in addition to also having data-races that rust would prevent. But, also, Go has way worse semantics around various things, like mutexes, making it much more likely deadlocks happen. Like in go, you see all sorts of "mu.Lock(); f(); mu.Unlock()" type code, where if it's called inside an `http.Handler` and 'f' panic…

> 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.

Trick question: what is the scope of `defer` in go?

Re: Go’s race detector has a mutex blind spot

#29

Post by judofyr »

mr90210 wrote: post #8
klabb3 wrote: post #4

Earlier quoted context omitted.

> Rewrite your code in rust, and get something better than the go race detector every time you compile. Congrats, rustc forced you to wrap all your types in Arc >, and you no longer have data races. As a gift, you will get logical race conditions instead, that are even more difficult to detect, while being equally difficult to reproduce reliably in unit tests and patch. Don’t get me wrong, Rust has done a ton for saf…

> Congrats, rustc forced you to wrap all your types in Arc > Also, don’t people know that a Mutex implies lower throughput depending on how long said Mutex is held? Lock-free data structures/algorithms are attempt to address the drawbacks of Mutexes. https://en.wikipedia.org/wiki/Lock_(computer_science)#Disadv...

Lock-free data structures does not guarantee higher throughput. They guarantee lower latency which often comes at the expense of the throughput. A typical approach for implementing a lock-free data structure is to allow one thread to "take over" the execution of another one by repeating parts of its work. It ensures progress of the system, even if one thread isn't being scheduled. This is mainly useful when you have CPUs competing for work running in parallel.

The performance of high-contention code is a really tricky to reason about and depends on a lot of factors. Just replacing a mutex with a lock-free data structure will not magically speed up your code. Eliminating the contention completely is typically much better in general.

Re: Go’s race detector has a mutex blind spot

#30

Post by tialaramex »

Xeoncross wrote: post #15

I'm so glad to be out of the dark ages of parallelism. Complaining about Go's race detector or exactly which types of logical races Rust can't prevent is such a breath of fresh air compared to all those other single-core languages we're paid to write with that had threading, async, or concurrency bolted-on as an afterthought. I can only hope Go and Rust continue to improve until the next language generation comes alo…

You know how a modern language like Rust doesn't have the unstructured control flow with features like "goto"† but only a set of structured control flow features, such as pattern matching, conditionals, loops and functions?

Structured Concurrency is the same idea, but for concurrency. Instead of that code to create an appropriate number of threads, parcel out work, and so on, you just express high level goals like "Do these N pieces of work in any order" or "Do A and B, and once either is finished also do C and D" and just as the language handles the actual machine code jumps for your control flow, that would happen for concurrency too.

Nothing as close to the metal as Rust has that baked in today, but it is beginning to be a thing in languages like Swift and you can find libraries which take this approach.

† C's goto is de-fanged from the full blown go-to arbitrary jump in early languages, but it's still not structured control flow.

Post reply on HN