Fix panic in handleSubscribeHTTP when client disconnects during publish
Replace wlock.TryLock() with a proper Lock() + closed flag to prevent writing to a response writer that has been cleaned up after the handler returns. The previous TryLock approach could not guarantee the response writer was still valid when a concurrent Publish goroutine called Flush.
This commit is contained in:
@@ -1458,12 +1458,15 @@ func (s *Server) handleSubscribeHTTP(w http.ResponseWriter, r *http.Request, v *
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
var wlock sync.Mutex
|
var wlock sync.Mutex
|
||||||
|
var closed bool
|
||||||
defer func() {
|
defer func() {
|
||||||
// Hack: This is the fix for a horrible data race that I have not been able to figure out in quite some time.
|
// This blocks until any in-flight sub() call finishes writing/flushing the response writer,
|
||||||
// It appears to be happening when the Go HTTP code reads from the socket when closing the request (i.e. AFTER
|
// then marks the connection as closed so future sub() calls are no-ops. This prevents a panic
|
||||||
// this function returns), and causes a data race with the ResponseWriter. Locking wlock here silences the
|
// from writing to a response writer that has been cleaned up after the handler returns.
|
||||||
// data race detector. See https://github.com/binwiederhier/ntfy/issues/338#issuecomment-1163425889.
|
// See https://github.com/binwiederhier/ntfy/issues/338#issuecomment-1163425889.
|
||||||
wlock.TryLock()
|
wlock.Lock()
|
||||||
|
closed = true
|
||||||
|
wlock.Unlock()
|
||||||
}()
|
}()
|
||||||
sub := func(v *visitor, msg *message) error {
|
sub := func(v *visitor, msg *message) error {
|
||||||
if !filters.Pass(msg) {
|
if !filters.Pass(msg) {
|
||||||
@@ -1475,6 +1478,9 @@ func (s *Server) handleSubscribeHTTP(w http.ResponseWriter, r *http.Request, v *
|
|||||||
}
|
}
|
||||||
wlock.Lock()
|
wlock.Lock()
|
||||||
defer wlock.Unlock()
|
defer wlock.Unlock()
|
||||||
|
if closed {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
if _, err := w.Write([]byte(m)); err != nil {
|
if _, err := w.Write([]byte(m)); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user