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

31–40 of 162 posts

Re: Conc: Better Structured Concurrency for Go

#31

Post by morelisp »

alexeldeib wrote: post #28
morelisp wrote: post #24

Earlier quoted context omitted.

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

> So you've never written code with a bug?

No, I write bugs all the fucking time! That's why I want my code to panic, so I know I wrote a bug. If that shit gets stashed away in a 500 response in a Prometheus metric maybe I'll chance to see it three days later. But if the container gets blown away, I'll notice.

Re: Conc: Better Structured Concurrency for Go

#32

Post by preseinger »

alexeldeib wrote: post #28
morelisp wrote: post #24

Earlier quoted context omitted.

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

Crashing the program _is_ the defensive posture. Panics -- concurrent map writes or nil pointer dereferences or almost anything else -- usually mean the program state has become invalid. You can't treat them like errors.

Re: Conc: Better Structured Concurrency for Go

#33

Post by gpderetta »

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.

On a memory unsafe language if you find your invariants violated all bets are off and it is possible that the integrity of the runtime is compromised. The only safe option is to bail out to the nearest protection boundary (i.e. the process).

On a memory safe language, the blast radius is potentially much smaller (in principle all objects transitively reachable from the failure point, but this is very pessimistic), so it is realistic to be able to isolate the failure in process.

Re: Conc: Better Structured Concurrency for Go

#34

Post by preseinger »

gpderetta wrote: post #33
morelisp wrote: post #24

Earlier quoted context omitted.

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

On a memory unsafe language if you find your invariants violated all bets are off and it is possible that the integrity of the runtime is compromised. The only safe option is to bail out to the nearest protection boundary (i.e. the process). On a memory safe language, the blast radius is potentially much smaller (in principle all objects transitively reachable from the failure point, but this is very pessimistic), so…

Well, memory safety is not a boolean, it's a property that's defined by the memory model of the language. That model also defines the impact of a violation.

Re: Conc: Better Structured Concurrency for Go

#35

Post by BreakfastB0b »

preseinger wrote: post #29
camdencheek wrote: post #18

Earlier quoted context omitted.

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

I would agree if it weren’t super easy to cause a panic in go.

Index slice out of bounds? panic. Close a channel twice? Panic. Incorrect type assertion? Panic. Dereference nil pointer? Panic.

I would argue that all of these examples which are the most common in my experience are “goroutine scoped” because the goroutine was aborted before they potentially modified the application state in an unknown way.

It’s like not in C, or C++ where out of bounds access has now put the entire application into an unknown state.

Re: Conc: Better Structured Concurrency for Go

#36

Post by preseinger »

klodolph wrote: post #30
morelisp wrote: post #27

Earlier quoted context omitted.

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()?

I think the OP is saying defer cancel() is correct, and the stuff people would do to avoid it are the things that are bad.

Re: Conc: Better Structured Concurrency for Go

#37

Post by ilyt »

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…

Also if that is your concern you'd just wrap the panic on your own anyway.

Re: Conc: Better Structured Concurrency for Go

#38

Post by preseinger »

camdencheek wrote: post #9

Hi! Author here. Conc is the result of generalizing and cleaning up an internal package I wrote for use within Sourcegraph. Basically, I got tired of rewriting code that handled panics, limited concurrency, and ensured goroutine cleanup. Happy to answer questions or address comments.

It's a good package in general, save for the panic handling. Panics should not be handled in this way. Remove that wart, and it's solid.

Re: Conc: Better Structured Concurrency for Go

#39

Post by camdencheek »

preseinger wrote: post #29
camdencheek wrote: post #18

Earlier quoted context omitted.

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

> panics aren't "goroutine scoped" in terms of their potential impact

I'm with ya there. However, there are also many classes of logic errors that are not goroutine-scoped. And there are many panics that do not have impact outside of the goroutine's scope. In my experience, this is true of most panics.

In practice, panics happen. They are (almost) always indicative of a bug, and almost always mean there is something that needs fixed. However, if a subsystem of my application is broken and panicking, there's a pretty good chance that reporting the panic without crashing the process will provide a better end user experience than just blowing up.

Yes, that means I'm accepting the risk that my application is left in an inconsistent state, but coupled with good observability/reporting, that's a tradeoff I'm willing to make.

(bonus: this is especially true when propagating panics allow me to capture more debugging information to fix the panics faster)

Re: Conc: Better Structured Concurrency for Go

#40

Post by preseinger »

BreakfastB0b wrote: post #35
preseinger wrote: post #29

Earlier quoted context omitted.

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

I would agree if it weren’t super easy to cause a panic in go. Index slice out of bounds? panic. Close a channel twice? Panic. Incorrect type assertion? Panic. Dereference nil pointer? Panic. I would argue that all of these examples which are the most common in my experience are “goroutine scoped” because the goroutine was aborted before they potentially modified the application state in an unknown way. It’s like not…

> Index slice out of bounds? panic. Close a channel twice? Panic. Incorrect type assertion? Panic. Dereference nil pointer? Panic.

These are all really bad things which should never survive to production code. It is not difficult to detect and prevent them.

> I would argue that all of these examples which are the most common in my experience are “goroutine scoped” because the goroutine was aborted before they potentially modified the application state in an unknown way.

What makes you think that terminating the goroutine that triggered these panics prevents them from impacting the process state?

> It’s like not in C, or C++ where out of bounds access has now put the entire application into an unknown state.

What makes you think this is the case? Panics have unknowable impact, and many panics (e.g. data races) absolutely do put the program into an unknown state.

Post reply on HN