Skip to content

Commit

Permalink
make EbpfTracker.dead go-routine-safe and .stop() idempotent
Browse files Browse the repository at this point in the history
Without synchronisation, the isDead() call might return a stale value,
delaying deadness detection potentially indefinitely.

Without the guards / idempotence in .stop(), invoking stop() more than
once could cause a panic, since tracer.Stop() closes a channel (which
panics on a closed channel). Multiple stop() invocations are rare, but
not impossible.
  • Loading branch information
rade committed Jul 11, 2017
1 parent cf63533 commit d568c50
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions probe/endpoint/ebpf.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ func (t *EbpfTracker) tcpEventCbV4(e tracer.TcpV4) {
// https://github.com/iovisor/bcc/issues/790#issuecomment-263704235
// https://github.com/weaveworks/scope/issues/2334
log.Errorf("tcp tracer received event with timestamp %v even though the last timestamp was %v. Stopping the eBPF tracker.", e.Timestamp, t.lastTimestampV4)
t.dead = true
t.stop()
}

Expand All @@ -118,7 +117,6 @@ func (t *EbpfTracker) tcpEventCbV6(e tracer.TcpV6) {

func (t *EbpfTracker) lostCb(count uint64) {
log.Errorf("tcp tracer lost %d events. Stopping the eBPF tracker", count)
t.dead = true
t.stop()
}

Expand Down Expand Up @@ -270,12 +268,17 @@ func (t *EbpfTracker) isReadyToHandleConnections() bool {
}

func (t *EbpfTracker) isDead() bool {
t.Lock()
defer t.Unlock()
return t.dead
}

func (t *EbpfTracker) stop() {
if t.tracer != nil {
t.Lock()
alreadyDead := t.dead
t.dead = true
t.Unlock()
if !alreadyDead && t.tracer != nil {
t.tracer.Stop()
}
t.dead = true
}

0 comments on commit d568c50

Please sign in to comment.