amahi / spdy

A production-ready SPDY library for building clients and servers, in Go
Other
95 stars 21 forks source link

runtime error: send on closed channel #16

Open cpg opened 10 years ago

cpg commented 10 years ago

We have seen an error of this kind. It took several days into seeing this error in a production system. It's not readily reproducible. In any case, this is what happens:

panic: runtime error: send on closed channel
goroutine 657610 [running]:
runtime.panic(0x65ccc0, 0x9589fe)
/usr/lib/golang/src/pkg/runtime/panic.c:266 +0xb6
github.com/amahi/spdy.(*Stream).WriteHeader(0xc22e410e80, 0x130)
src/github.com/amahi/spdy/stream.go:558 +0x4e2
github.com/amahi/spdy.(*Stream).handleSynReply(0xc2ec6fd080, 0x2, 0xc2ebcf79e0, 0x17, 0x17, ...)
src/github.com/amahi/spdy/stream.go:591 +0x4df
github.com/amahi/spdy.(*Stream).stream_loop(0xc2ec6fd080, 0x0, 0x0)
src/github.com/amahi/spdy/stream.go:461 +0x22c
github.com/amahi/spdy.(*Stream).serve(0xc2ec6fd080)
src/github.com/amahi/spdy/stream.go:422 +0xf7
created by github.com/amahi/spdy.(*Session).NewClientStream
src/github.com/amahi/spdy/stream.go:45 +0x1d0
cpg commented 9 years ago

This continues to be a problem. When something happens upstream and things get disconnected at any point, some goroutines are left dangling. We need better error caching for non-mainstream cases like this.

wallnutkraken commented 7 years ago

I'm not too familiar with the library and I've kinda just spent an hour looking around it, but what it looks like to me is that in stream.go:558 (where panic happens) there is a missing check for if the session is closed:

...
// Write the frame
    sr := frameSynReply{session: s.session, stream: s.id, headers: s.headers}
    debug.Println("Sending SYN_REPLY", sr)
    s.session.out <- sr
    s.wroteHeader = true
...

If the stacktrace is pointing to the correct lines of code, then it might be the case. A check if s.session.closed is true might be what this needs, but this method does not return an error, so I'm not sure how that should be approached. Either way, I'm not sure if there is anything in the code that can cause the session to be closed before WriteHeader is called. Perhaps someone who knows the code better could answer that :)