From d568c50ec4595a75db17425423828d6e7c9f7c30 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 10 Jul 2017 20:46:55 +0100 Subject: [PATCH] make EbpfTracker.dead go-routine-safe and .stop() idempotent 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. --- probe/endpoint/ebpf.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/probe/endpoint/ebpf.go b/probe/endpoint/ebpf.go index 9e9765f2a..f9d57dcec 100644 --- a/probe/endpoint/ebpf.go +++ b/probe/endpoint/ebpf.go @@ -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() } @@ -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() } @@ -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 }