Skip to content

Commit 50c5fb1

Browse files
mjuragaGopher Bot
authored andcommitted
BUG/MEDIUM: runtime: fix data race on lastError causing nil pointer panic
The lastError field was read by Listen() and written by the internal listen() goroutine without synchronization. When Close() was called externally (e.g. during HAProxy reload), the events channel could be closed before listen() had set lastError. The caller would then read a partially-written error interface value: a non-nil interface with a nil underlying *wrapError pointer. Calling errors.Is() on this would dereference the nil pointer in fmt.(*wrapError).Unwrap(). Fix by protecting lastError with a sync.Mutex via setLastError/ getLastError accessors. Also return io.EOF as a fallback when the events channel is closed but no error was recorded, instead of returning nil which callers would interpret as success.
1 parent 52d98dd commit 50c5fb1

File tree

1 file changed

+27
-9
lines changed

1 file changed

+27
-9
lines changed

runtime/runtime_event_listener.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ import (
2020
"bytes"
2121
"context"
2222
"fmt"
23+
"io"
2324
"net"
2425
"slices"
2526
"strings"
27+
"sync"
2628
"sync/atomic"
2729
"time"
2830
)
@@ -57,11 +59,12 @@ type EventListener struct {
5759
// Write timeout. Can be set before calling Listen.
5860
WriteTimeout time.Duration
5961
// Message delimiter. Either \n or 0 (zero).
60-
delim byte
61-
events chan Event
62-
done chan struct{}
63-
lastError error
64-
closed atomic.Bool
62+
delim byte
63+
events chan Event
64+
done chan struct{}
65+
errMu sync.Mutex
66+
lastErr error
67+
closed atomic.Bool
6568
}
6669

6770
// NewEventListener connects to HAProxy's master socket and returns a new
@@ -115,14 +118,29 @@ func (l *EventListener) Listen(ctx context.Context) (Event, error) {
115118
select {
116119
case event, ok := <-l.events:
117120
if !ok {
118-
return Event{}, l.lastError
121+
if err := l.getLastError(); err != nil {
122+
return Event{}, err
123+
}
124+
return Event{}, io.EOF
119125
}
120126
return event, nil
121127
case <-ctx.Done():
122128
return Event{}, l.Close()
123129
}
124130
}
125131

132+
func (l *EventListener) setLastError(err error) {
133+
l.errMu.Lock()
134+
l.lastErr = err
135+
l.errMu.Unlock()
136+
}
137+
138+
func (l *EventListener) getLastError() error {
139+
l.errMu.Lock()
140+
defer l.errMu.Unlock()
141+
return l.lastErr
142+
}
143+
126144
// Close the EventListener cleanly. The events channel will be closed
127145
// asynchronously by the internal listen goroutine after it exits.
128146
func (l *EventListener) Close() error {
@@ -147,22 +165,22 @@ func (l *EventListener) listen() {
147165

148166
_, err := fmt.Fprintf(l.conn, "@@1 set severity-output number;%s\n", l.listenCmd)
149167
if err != nil {
150-
l.lastError = l.errorf("%w", err)
168+
l.setLastError(l.errorf("%w", err))
151169
_ = l.Close()
152170
return
153171
}
154172

155173
for {
156174
data, err := l.reader.ReadBytes(l.delim)
157175
if err != nil {
158-
l.lastError = l.errorf("%w", err)
176+
l.setLastError(l.errorf("%w", err))
159177
_ = l.Close()
160178
return
161179
}
162180

163181
event, err := l.parseEvent(data)
164182
if err != nil {
165-
l.lastError = err
183+
l.setLastError(err)
166184
_ = l.Close()
167185
return
168186
}

0 commit comments

Comments
 (0)