Skip to content

Fix UpdateActivatedProbes panic, refactor state checks, use stopping state #290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gh-worker-dd-mergequeue-cf854d merged 6 commits into from
Sep 17, 2026
Merged

Fix UpdateActivatedProbes panic, refactor state checks, use stopping state #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1c79ced
Fix panic update probes
theop-dd Sep 8, 2026
b9fa436
refactor: replace state comparison operators with switch/case
theop-dd Sep 10, 2026
78cd5cc
block manager operations during stopping state
theop-dd Sep 10, 2026
1056ec8
Restore previous state to not stay on stopping after stopReaders
theop-dd Sep 14, 2026
f05e8ad
Check again status after stopReaders
theop-dd Sep 15, 2026
37ddcf1
Add go retract to skip broken versions
theop-dd Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ require (
golang.org/x/sync v0.22.0
golang.org/x/sys v0.47.0
)

retract (
[v0.8.5, v0.8.6] // these versions can panic
)
179 changes: 137 additions & 42 deletions manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,12 @@ type Manager struct {
func (m *Manager) DumpMaps(w io.Writer, maps ...string) error {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return ErrManagerNotInitialized
}

Expand Down Expand Up @@ -230,7 +235,12 @@ func (m *Manager) getMap(name string) (*ebpf.Map, bool, error) {
func (m *Manager) GetMap(name string) (*ebpf.Map, bool, error) {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return nil, false, ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return nil, false, ErrManagerNotInitialized
}
return m.getMap(name)
Expand All @@ -240,7 +250,12 @@ func (m *Manager) GetMap(name string) (*ebpf.Map, bool, error) {
func (m *Manager) GetMaps() (map[string]*ebpf.Map, error) {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return nil, ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return nil, ErrManagerNotInitialized
}

Expand Down Expand Up @@ -276,7 +291,12 @@ func (m *Manager) getMapSpec(name string) (*ebpf.MapSpec, bool, error) {
func (m *Manager) GetMapSpec(name string) (*ebpf.MapSpec, bool, error) {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collectionSpec == nil || m.state < elfLoaded {
switch m.state {
case reset:
return nil, false, ErrManagerNotELFLoaded
case elfLoaded, initialized, stopping, paused, running:
}
if m.collectionSpec == nil {
return nil, false, ErrManagerNotELFLoaded
}
return m.getMapSpec(name)
Expand Down Expand Up @@ -346,7 +366,12 @@ func (m *Manager) getProgram(id ProbeIdentificationPair) ([]*ebpf.Program, bool,
func (m *Manager) GetProgram(id ProbeIdentificationPair) ([]*ebpf.Program, bool, error) {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return nil, false, ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return nil, false, ErrManagerNotInitialized
}
return m.getProgram(id)
Expand All @@ -356,7 +381,12 @@ func (m *Manager) GetProgram(id ProbeIdentificationPair) ([]*ebpf.Program, bool,
func (m *Manager) GetPrograms() (map[string]*ebpf.Program, error) {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return nil, ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return nil, ErrManagerNotInitialized
}

Expand All @@ -367,7 +397,12 @@ func (m *Manager) GetPrograms() (map[string]*ebpf.Program, error) {
func (m *Manager) GetMapSpecs() (map[string]*ebpf.MapSpec, error) {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collectionSpec == nil || m.state < elfLoaded {
switch m.state {
case reset:
return nil, ErrManagerNotELFLoaded
case elfLoaded, initialized, stopping, paused, running:
}
if m.collectionSpec == nil {
return nil, ErrManagerNotELFLoaded
}

Expand All @@ -378,7 +413,12 @@ func (m *Manager) GetMapSpecs() (map[string]*ebpf.MapSpec, error) {
func (m *Manager) GetProgramSpecs() (map[string]*ebpf.ProgramSpec, error) {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collectionSpec == nil || m.state < elfLoaded {
switch m.state {
case reset:
return nil, ErrManagerNotELFLoaded
case elfLoaded, initialized, stopping, paused, running:
}
if m.collectionSpec == nil {
return nil, ErrManagerNotELFLoaded
}

Expand Down Expand Up @@ -415,7 +455,12 @@ func (m *Manager) getProgramSpec(id ProbeIdentificationPair) ([]*ebpf.ProgramSpe
func (m *Manager) GetProgramSpec(id ProbeIdentificationPair) ([]*ebpf.ProgramSpec, bool, error) {
m.stateLock.RLock()
defer m.stateLock.RUnlock()
if m.collectionSpec == nil || m.state < elfLoaded {
switch m.state {
case reset:
return nil, false, ErrManagerNotELFLoaded
case elfLoaded, initialized, stopping, paused, running:
}
if m.collectionSpec == nil {
return nil, false, ErrManagerNotELFLoaded
}
return m.getProgramSpec(id)
Expand Down Expand Up @@ -450,10 +495,13 @@ func (m *Manager) GetProbe(id ProbeIdentificationPair) (*Probe, bool) {
func (m *Manager) LoadELF(elf io.ReaderAt) error {
m.stateLock.Lock()
defer m.stateLock.Unlock()
if m.state >= elfLoaded {
switch m.state {
case reset:
return m.loadELF(elf)
case elfLoaded, initialized, stopping, paused, running:
return ErrManagerELFLoaded
}
return m.loadELF(elf)
return nil
}

func (m *Manager) loadELF(elf io.ReaderAt) error {
Expand Down Expand Up @@ -495,8 +543,10 @@ func (m *Manager) initState(elf io.ReaderAt, options Options) error {
m.stateLock.Lock()
defer m.stateLock.Unlock()

if m.state > initialized {
switch m.state {
case stopping, paused, running:
return ErrManagerRunning
case reset, elfLoaded, initialized:
}

m.options = options
Expand All @@ -520,16 +570,18 @@ func (m *Manager) initState(elf io.ReaderAt, options Options) error {
}
}

if m.state < elfLoaded {
switch m.state {
case reset:
if elf == nil {
return fmt.Errorf("nil ELF reader")
}

if err := m.loadELF(elf); err != nil {
return err
}
} else if elf != nil {
return ErrManagerELFLoaded
case elfLoaded, initialized, stopping, paused, running:
if elf != nil {
return ErrManagerELFLoaded
}
}

if m.options.AdditionalExcludedFunctionCollector != nil {
Expand Down Expand Up @@ -788,13 +840,14 @@ func (m *Manager) releaseKernelBTF() {
// Start - Attach eBPF programs, start perf ring readers and apply maps and tail calls routing.
func (m *Manager) Start() error {
m.stateLock.Lock()
if m.state < initialized {
switch m.state {
case reset, elfLoaded, stopping:
m.stateLock.Unlock()
return ErrManagerNotInitialized
}
if m.state >= running {
case running:
m.stateLock.Unlock()
return nil
case initialized, paused:
}

// release kernel BTF: it is only needed while loading programs and should no longer be needed now
Expand Down Expand Up @@ -874,11 +927,12 @@ func (m *Manager) Start() error {
func (m *Manager) Pause() error {
m.stateLock.Lock()
defer m.stateLock.Unlock()
if m.state == paused {
switch m.state {
case paused:
return nil
}
if m.state <= initialized {
case reset, elfLoaded, initialized, stopping:
return ErrManagerNotStarted
case running:
}
if !m.options.BypassEnabled {
return nil
Expand All @@ -896,11 +950,12 @@ func (m *Manager) Pause() error {
func (m *Manager) Resume() error {
m.stateLock.Lock()
defer m.stateLock.Unlock()
if m.state == running {
switch m.state {
case running:
return nil
}
if m.state <= initialized {
case reset, elfLoaded, initialized, stopping:
return ErrManagerNotStarted
case paused:
}
if !m.options.BypassEnabled {
return nil
Expand All @@ -920,22 +975,33 @@ func (m *Manager) Resume() error {
func (m *Manager) Stop(cleanup MapCleanupType) error {
m.stateLock.Lock()
defer m.stateLock.Unlock()
if m.state < initialized {
switch m.state {
case reset, elfLoaded:
return ErrManagerNotInitialized
case initialized, stopping, paused, running:

@kovagsm kovagsm Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's stopping, it shouldn't stop again

@theop-dd theop-dd Sep 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stopping is only a window, not a terminal state.

stopReaders sets it before dropping the lock (so Start/NewPerfRing/NewRingBuffer cannot add readers while we Wait), then restores the previous state before returning — unless a concurrent Stop() already moved the manager to reset. That way StopReaders() does not leave the manager stuck in stopping.

Stop() still accepts stopping for the concurrent case: a Stop() that arrives during that unlock window. After StopReaders() returns, state is restored, so the usual StopReaders() then Stop() sequence sees initialized/paused/running again. closeReader is idempotent if the readers were already shut down.

// stopping is only visible while another stopReaders holds the unlock
// window. After StopReaders returns, previousState is restored, so a
// later Stop() sees initialized/paused/running. Accepting stopping here
// covers the concurrent Stop() that arrives during that window.
return m.stop(cleanup)
}
return m.stop(cleanup)
return nil
}

// StopReaders stop the kernel events readers Perf or Ring buffer.
// It is not safe to call NewPerfRing or NewRingBuffer concurrently
// with StopReaders since we cannot put state to reset here.
// StopReaders - Stop the kernel events readers Perf or Ring buffer.
func (m *Manager) StopReaders(cleanup MapCleanupType) error {
m.stateLock.Lock()
defer m.stateLock.Unlock()
return m.stopReaders(cleanup)
}

// stopReaders - Thread unsafe version of StopReaders. The caller must hold stateLock.
func (m *Manager) stopReaders(cleanup MapCleanupType) error {
// Block Start/NewPerfRing/NewRingBuffer for the unlock window below, then
// restore so StopReaders is not a sticky state. Only restore if we are
// still stopping: a concurrent Stop() may already have gone to reset.
previousState := m.state
m.state = stopping
var errs []error

// Stop perf ring readers
Expand Down Expand Up @@ -981,6 +1047,9 @@ func (m *Manager) stopReaders(cleanup MapCleanupType) error {
}
}

if m.state == stopping {
m.state = previousState
}
return errors.Join(errs...)
}

Expand All @@ -1000,15 +1069,18 @@ func (m *Manager) stopProbes() error {
return eg.Wait()
}

// stop - Thread unsafe version of Stop. Requires the manager lock to not panic
func (m *Manager) stop(cleanup MapCleanupType) error {
// Set state to reset early, to prevent concurrent operations (like
// NewPerfRing, NewRingBuffer) from adding new readers during the
// unlock window.
m.state = reset

var errs []error
errs = append(errs, m.stopReaders(cleanup))

// check again state since stopReaders released the lock temporary
switch m.state {
case reset, elfLoaded:
return ErrManagerNotInitialized
case initialized, stopping, paused, running:
}

// Detach eBPF programs
errs = append(errs, m.stopProbes())

Expand All @@ -1027,16 +1099,22 @@ func (m *Manager) stop(cleanup MapCleanupType) error {
// situations. We can't rely only on the collection to close all maps and programs because some pinned objects were
// removed from the collection.
m.collection.Close()

m.state = reset
return errors.Join(errs...)

}

// NewMap - Create a new map using the provided parameters. The map is added to the list of maps managed by the manager.
// Use a MapRoute to make this map available to the programs of the manager.
func (m *Manager) NewMap(spec *ebpf.MapSpec, options MapOptions) (*ebpf.Map, error) {
m.stateLock.Lock()
defer m.stateLock.Unlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return nil, ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return nil, ErrManagerNotInitialized
}

Expand Down Expand Up @@ -1090,7 +1168,12 @@ func (m *Manager) CloneMap(name string, newName string, options MapOptions) (*eb
func (m *Manager) AddHook(UID string, newProbe *Probe) error {
m.stateLock.Lock()
defer m.stateLock.Unlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return ErrManagerNotInitialized
}

Expand Down Expand Up @@ -1176,7 +1259,12 @@ func (m *Manager) AddHook(UID string, newProbe *Probe) error {
func (m *Manager) DetachHook(id ProbeIdentificationPair) error {
m.stateLock.Lock()
defer m.stateLock.Unlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return ErrManagerNotInitialized
}

Expand Down Expand Up @@ -1225,7 +1313,12 @@ func (m *Manager) CloneProgram(UID string, newProbe *Probe, constantsEditors []C
func (m *Manager) CloneProgramWithSpecEditor(UID string, newProbe *Probe, constantsEditors []ConstantEditor, mapEditors map[string]*ebpf.Map, specEditor func(spec *ebpf.ProgramSpec)) error {
m.stateLock.Lock()
defer m.stateLock.Unlock()
if m.collection == nil || m.state < initialized {
switch m.state {
case reset, elfLoaded:
return ErrManagerNotInitialized
case initialized, stopping, paused, running:
}
if m.collection == nil {
return ErrManagerNotInitialized
}

Expand Down Expand Up @@ -1470,9 +1563,11 @@ func (m *Manager) activateProbes() {
// UpdateActivatedProbes - update the list of activated probes
func (m *Manager) UpdateActivatedProbes(selectors []ProbesSelector) error {
m.stateLock.Lock()
if m.state < initialized {
switch m.state {
case reset, elfLoaded:
m.stateLock.Unlock()
return ErrManagerNotInitialized
case initialized, stopping, paused, running:
}

currentProbes := make(map[ProbeIdentificationPair]*Probe)
Expand Down Expand Up @@ -1537,7 +1632,7 @@ func (m *Manager) UpdateActivatedProbes(selectors []ProbesSelector) error {

if validationErrs != nil {
// Clean up
_ = m.stop(CleanInternal)
_ = m.Stop(CleanInternal)
return fmt.Errorf("probes activation validation failed: %w", validationErrs)
}

Expand Down
Loading
Loading