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

Conc: Better Structured Concurrency for Go

github.com

Post reply on HN
Print view View original

21–30 of 162 posts

Re: Conc: Better Structured Concurrency for Go

#21

Post by avita1 »

kevmo314 wrote: post #7

The WaitGroup looks suspiciously like errgroup, which even has the .WithMaxGoroutines() functionality: https://pkg.go.dev/golang.org/x/sync/errgroup > A frequent problem with goroutines in long-running applications is handling panics. A goroutine spawned without a panic handler will crash the whole process on panic. This is usually undesirable. In go land, this seems desirable. Recoverable errors should be propagated…

This isn't catching the panic though, this is propagating the panic through the parent goroutine. The whole program will still shut down, but the stacktrace that shows up in the panic contains not only information information about the goroutine that panicked, but also the launching goroutine. That can help you figure out why the panic happened to begin with.

Re: Conc: Better Structured Concurrency for Go

#22

Post by klodolph »

jerf wrote: post #14
kevmo314 wrote: post #7

The WaitGroup looks suspiciously like errgroup, which even has the .WithMaxGoroutines() functionality: https://pkg.go.dev/golang.org/x/sync/errgroup > A frequent problem with goroutines in long-running applications is handling panics. A goroutine spawned without a panic handler will crash the whole process on panic. This is usually undesirable. In go land, this seems desirable. Recoverable errors should be propagated…

It is the way of things in an imperative language. If you catch a panic, you are also declaring to the runtime that there is nothing dangling, no locks in a bad state, etc. This is often the case. (Although since I don't think this is a well-understood aspect of what catching a panic means, it is arguably only usually true by a certain amount of coincidence.) But if you don't say that to the runtime, it can't assume…

I don’t think this is really a question of whether your code is imperative, since Haskell code will terminate just as surely as Go code if you try to access an array element out of range.

(Haskell’s lazy evaluation just makes it a bit harder to catch, since you need to force evaluation of the thunk within the catch statement, and it’s far too easy to end up passing your thunk to somebody who won’t catch the exception.)

As a matter of Go style, of course, you should almost always defer unlock() after you lock(), but some people sometimes get clever and think that they can just lock() and unlock() manually without using defer. This is not hypothetical, and it causes other problems besides leaving dangling locks after a panic(). Somebody sticks a “return” between lock() and unlock(), without noticing, for example.

So my impression of catching panic() is that it is about as safe as not catching panic(). What I mean by that is that if recover() is not safe in your code base, there is a good chance that there are other, related bugs in your code base, and being a bit more strict about using defer and not trying to be clever will go a long way.

Re: Conc: Better Structured Concurrency for Go

#23

Post by morelisp »

camdencheek wrote: post #18
kevmo314 wrote: post #7

The WaitGroup looks suspiciously like errgroup, which even has the .WithMaxGoroutines() functionality: https://pkg.go.dev/golang.org/x/sync/errgroup > A frequent problem with goroutines in long-running applications is handling panics. A goroutine spawned without a panic handler will crash the whole process on panic. This is usually undesirable. In go land, this seems desirable. Recoverable errors should be propagated…

> The WaitGroup looks suspiciously like errgroup I heavily used errgroup before creating conc, so the design is likely strongly influenced by that of errgroup even if not consciously. Conc was partially built to address the shortcomings of errgroup (from my perspective). Probably worth adding a "prior art" section to the README, but many of the ideas in conc are not unique. > In go land, this seems desirable. I mostl…

> This keeps panics scoped to the spawning goroutine

This is exactly what's undesirable. (IMO and from my reading the GP agrees.)

Re: Conc: Better Structured Concurrency for Go

#24

Post by morelisp »

alexeldeib wrote: post #12
kevmo314 wrote: post #7

The WaitGroup looks suspiciously like errgroup, which even has the .WithMaxGoroutines() functionality: https://pkg.go.dev/golang.org/x/sync/errgroup > A frequent problem with goroutines in long-running applications is handling panics. A goroutine spawned without a panic handler will crash the whole process on panic. This is usually undesirable. In go land, this seems desirable. Recoverable errors should be propagated…

A goroutine created inside an http request handler (itself a goroutine) which then panics, by default will crash the whole server, not the single request. The panic could simply be an out of bounds access. That should not crash the whole server. It’s a logic bug, but you can’t “not panic”. You can trap and recover it though. Bit orthogonal to OP but relevant to your reasoning.

> simply be an out of bounds access

If you have any care for quality at all, there's nothing "simple" about your invariants being violated.

Re: Conc: Better Structured Concurrency for Go

#25

Post by morelisp »

klodolph wrote: post #22
jerf wrote: post #14

Earlier quoted context omitted.

It is the way of things in an imperative language. If you catch a panic, you are also declaring to the runtime that there is nothing dangling, no locks in a bad state, etc. This is often the case. (Although since I don't think this is a well-understood aspect of what catching a panic means, it is arguably only usually true by a certain amount of coincidence.) But if you don't say that to the runtime, it can't assume…

I don’t think this is really a question of whether your code is imperative, since Haskell code will terminate just as surely as Go code if you try to access an array element out of range. (Haskell’s lazy evaluation just makes it a bit harder to catch, since you need to force evaluation of the thunk within the catch statement, and it’s far too easy to end up passing your thunk to somebody who won’t catch the exception…

Since the Go 1.14 optimizations I don't believe I've found a single case where `X(); defer Unx()` has been worse.

Unfortunately this was not the case before 1.14, so there's a lot of "middle-aged" code floating around setting a bad example.

Re: Conc: Better Structured Concurrency for Go

#26

Post by klodolph »

morelisp wrote: post #25
klodolph wrote: post #22

Earlier quoted context omitted.

I don’t think this is really a question of whether your code is imperative, since Haskell code will terminate just as surely as Go code if you try to access an array element out of range. (Haskell’s lazy evaluation just makes it a bit harder to catch, since you need to force evaluation of the thunk within the catch statement, and it’s far too easy to end up passing your thunk to somebody who won’t catch the exception…

Since the Go 1.14 optimizations I don't believe I've found a single case where `X(); defer Unx()` has been worse. Unfortunately this was not the case before 1.14, so there's a lot of "middle-aged" code floating around setting a bad example.

Old habits die hard. For what it’s worth, the bugs I saw were long before the 1.14 release. Somewhere around 1.7 or something.

Re: Conc: Better Structured Concurrency for Go

#27

Post by morelisp »

klodolph wrote: post #26
morelisp wrote: post #25

Earlier quoted context omitted.

Since the Go 1.14 optimizations I don't believe I've found a single case where `X(); defer Unx()` has been worse. Unfortunately this was not the case before 1.14, so there's a lot of "middle-aged" code floating around setting a bad example.

Old habits die hard. For what it’s worth, the bugs I saw were long before the 1.14 release. Somewhere around 1.7 or something.

Agreed - it's also not nearly as clear for `ctx, cancel := ...; defer cancel()` and I see that reflex in a few places where it's not only often much less efficient, but logically wrong / less safe.

Re: Conc: Better Structured Concurrency for Go

#28

Post by alexeldeib »

morelisp wrote: post #24
alexeldeib wrote: post #12

Earlier quoted context omitted.

A goroutine created inside an http request handler (itself a goroutine) which then panics, by default will crash the whole server, not the single request. The panic could simply be an out of bounds access. That should not crash the whole server. It’s a logic bug, but you can’t “not panic”. You can trap and recover it though. Bit orthogonal to OP but relevant to your reasoning.

> simply be an out of bounds access If you have any care for quality at all, there's nothing "simple" about your invariants being violated.

So you've never written code with a bug? There are other ways to panic in go - concurrent map writes, nil pointer dereference. I'm not saying it should happen, but best practice would be a defensive posture especially when it's effectively zero cost, not hoping for the best.

Re: Conc: Better Structured Concurrency for Go

#29

Post by preseinger »

camdencheek wrote: post #18
kevmo314 wrote: post #7

The WaitGroup looks suspiciously like errgroup, which even has the .WithMaxGoroutines() functionality: https://pkg.go.dev/golang.org/x/sync/errgroup > A frequent problem with goroutines in long-running applications is handling panics. A goroutine spawned without a panic handler will crash the whole process on panic. This is usually undesirable. In go land, this seems desirable. Recoverable errors should be propagated…

> The WaitGroup looks suspiciously like errgroup I heavily used errgroup before creating conc, so the design is likely strongly influenced by that of errgroup even if not consciously. Conc was partially built to address the shortcomings of errgroup (from my perspective). Probably worth adding a "prior art" section to the README, but many of the ideas in conc are not unique. > In go land, this seems desirable. I mostl…

> That said, crashing the whole webserver because of one misbehaving request is not necessarily a good tradeoff. Conc moves panics into the spawning goroutine, which makes it possible to do things like catch panics at the top of a request and return a useful error to the caller, even if that error is just "nil pointer dereference" with a stacktrace. It's up to the user to decide what to do with propagated panics.

The problem is that panics aren't "goroutine scoped" in terms of their potential impact. So it really shouldn't be up to the user to decide how to handle a panic. Application code shouldn't handle panics at all! They're not just a different way to yield an error, they're critical bugs which shouldn't occur at all.

Re: Conc: Better Structured Concurrency for Go

#30

Post by klodolph »

morelisp wrote: post #27
klodolph wrote: post #26

Earlier quoted context omitted.

Old habits die hard. For what it’s worth, the bugs I saw were long before the 1.14 release. Somewhere around 1.7 or something.

Agreed - it's also not nearly as clear for `ctx, cancel := ...; defer cancel()` and I see that reflex in a few places where it's not only often much less efficient, but logically wrong / less safe.

What issues do you see with defer cancel()?
Post reply on HN