Skip to content

Commit

Permalink
Merge pull request #1668 from safing/fix/spn-start-stop
Browse files Browse the repository at this point in the history
Fix starting/stopping SPN + more
  • Loading branch information
dhaavi authored Aug 28, 2024
2 parents 2fd7c61 + 57e81fb commit 91f4fe5
Show file tree
Hide file tree
Showing 22 changed files with 718 additions and 59 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ jobs:
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v4
with:
version: v1.57.1
version: v1.60.3
only-new-issues: true
args: -c ./.golangci.yml --timeout 15m

Expand Down
7 changes: 4 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ linters:
- gocyclo
- goerr113
- gomnd
- gomoddirectives
- ifshort
- interfacebloat
- interfacer
- ireturn
- lll
- mnd
- musttag
- nestif
- nilnil
Expand All @@ -31,16 +33,15 @@ linters:
- nolintlint
- nonamedreturns
- nosnakecase
- perfsprint # TODO(ppacher): we should re-enanble this one to avoid costly fmt.* calls in the hot-path
- revive
- tagliatelle
- testifylint
- testpackage
- varnamelen
- whitespace
- wrapcheck
- wsl
- perfsprint # TODO(ppacher): we should re-enanble this one to avoid costly fmt.* calls in the hot-path
- testifylint
- gomoddirectives

linters-settings:
revive:
Expand Down
17 changes: 7 additions & 10 deletions base/rng/tickfeeder.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,15 @@ func tickFeeder(ctx *mgr.WorkerCtx) error {
feeder := NewFeeder()
defer feeder.CloseFeeder()

tickDuration := getTickFeederTickDuration()
ticker := time.NewTicker(getTickFeederTickDuration())
defer ticker.Stop()

for {
// wait for tick
time.Sleep(tickDuration)
select {
case <-ticker.C:
case <-ctx.Done():
return nil
}

// add tick value
value = (value << 1) | (time.Now().UnixNano() % 2)
Expand All @@ -64,13 +68,6 @@ func tickFeeder(ctx *mgr.WorkerCtx) error {
case <-ctx.Done():
return nil
}
} else {
// check if are done
select {
case <-ctx.Done():
return nil
default:
}
}
}
}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ require (
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/josharian/native v1.1.0 // indirect
github.com/klauspost/cpuid/v2 v2.2.8 // indirect
github.com/maruel/panicparse/v2 v2.3.1 // indirect
github.com/mdlayher/netlink v1.7.2 // indirect
github.com/mdlayher/socket v0.5.1 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
Expand Down
7 changes: 7 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,16 @@ github.com/lmittmann/tint v1.0.5 h1:NQclAutOfYsqs2F1Lenue6OoWCajs5wJcP3DfWVpePw=
github.com/lmittmann/tint v1.0.5/go.mod h1:HIS3gSy7qNwGCj+5oRjAutErFBl4BzdQP6cJZ0NfMwE=
github.com/magiconair/properties v1.7.4-0.20170902060319-8d7837e64d3c/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/maruel/panicparse/v2 v2.3.1 h1:NtJavmbMn0DyzmmSStE8yUsmPZrZmudPH7kplxBinOA=
github.com/maruel/panicparse/v2 v2.3.1/go.mod h1:s3UmQB9Fm/n7n/prcD2xBGDkwXD6y2LeZnhbEXvs9Dg=
github.com/mat/besticon v3.12.0+incompatible h1:1KTD6wisfjfnX+fk9Kx/6VEZL+MAW1LhCkL9Q47H9Bg=
github.com/mat/besticon v3.12.0+incompatible/go.mod h1:mA1auQYHt6CW5e7L9HJLmqVQC8SzNk2gVwouO0AbiEU=
github.com/mattn/go-colorable v0.0.10-0.20170816031813-ad5389df28cd/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4=
github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
github.com/mattn/go-isatty v0.0.2/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
Expand All @@ -205,6 +209,7 @@ github.com/mdlayher/socket v0.1.0/go.mod h1:mYV5YIZAfHh4dzDVzI8x8tWLWCliuX8Mon5A
github.com/mdlayher/socket v0.1.1/go.mod h1:mYV5YIZAfHh4dzDVzI8x8tWLWCliuX8Mon5Awbj+qDs=
github.com/mdlayher/socket v0.5.1 h1:VZaqt6RkGkt2OE9l3GcC6nZkqD3xKeQLyfleW/uBcos=
github.com/mdlayher/socket v0.5.1/go.mod h1:TjPLHI1UgwEv5J1B5q0zTZq12A/6H7nKmtTanQE37IQ=
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE=
github.com/miekg/dns v1.1.62 h1:cN8OuEF1/x5Rq6Np+h1epln8OiyPWV+lROx9LxcGgIQ=
github.com/miekg/dns v1.1.62/go.mod h1:mvDlcItzm+br7MToIKqkglaGhlFMHJ9DTNNWONWXbNQ=
github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw=
Expand Down Expand Up @@ -402,13 +407,15 @@ golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210525143221-35b2ab0089ea/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210906170528-6f6e22806c34/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211210111614-af8b64212486/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220128215802-99c3d69c2c27/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220408201424-a24fb2fb8a0f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20221010170243-090e33056c14/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
Expand Down
1 change: 1 addition & 0 deletions service/core/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ func debugInfo(ar *api.Request) (data []byte, err error) {
// Detailed information.
updates.AddToDebugInfo(di)
compat.AddToDebugInfo(di)
module.instance.AddWorkerInfoToDebugInfo(di)
di.AddGoroutineStack()

// Return data.
Expand Down
2 changes: 2 additions & 0 deletions service/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/safing/portmaster/base/log"
"github.com/safing/portmaster/base/metrics"
"github.com/safing/portmaster/base/utils/debug"
_ "github.com/safing/portmaster/service/broadcasts"
"github.com/safing/portmaster/service/mgr"
_ "github.com/safing/portmaster/service/netenv"
Expand Down Expand Up @@ -112,4 +113,5 @@ func New(instance instance) (*Core, error) {

type instance interface {
Shutdown()
AddWorkerInfoToDebugInfo(di *debug.Info)
}
64 changes: 64 additions & 0 deletions service/debug.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package service

import (
"bytes"
"errors"
"fmt"
"io"
"runtime"

"github.com/maruel/panicparse/v2/stack"

"github.com/safing/portmaster/base/utils/debug"
"github.com/safing/portmaster/service/mgr"
)

// GetWorkerInfo returns the worker info of all running workers.
func (i *Instance) GetWorkerInfo() (*mgr.WorkerInfo, error) {
snapshot, _, err := stack.ScanSnapshot(bytes.NewReader(fullStack()), io.Discard, stack.DefaultOpts())
if err != nil && !errors.Is(err, io.EOF) {
return nil, fmt.Errorf("get stack: %w", err)
}

infos := make([]*mgr.WorkerInfo, 0, 32)
for _, m := range i.serviceGroup.Modules() {
wi, _ := m.Manager().WorkerInfo(snapshot) // Does not fail when we provide a snapshot.
infos = append(infos, wi)
}
for _, m := range i.SpnGroup.Modules() {
wi, _ := m.Manager().WorkerInfo(snapshot) // Does not fail when we provide a snapshot.
infos = append(infos, wi)
}

return mgr.MergeWorkerInfo(infos...), nil
}

// AddWorkerInfoToDebugInfo adds the worker info of all running workers to the debug info.
func (i *Instance) AddWorkerInfoToDebugInfo(di *debug.Info) {
info, err := i.GetWorkerInfo()
if err != nil {
di.AddSection(
"Worker Status Failed",
debug.UseCodeSection,
err.Error(),
)
return
}

di.AddSection(
fmt.Sprintf("Worker Status: %d/%d (%d?)", info.Running, len(info.Workers), info.Missing+info.Other),
debug.UseCodeSection,
info.Format(),
)
}

func fullStack() []byte {
buf := make([]byte, 8096)
for {
n := runtime.Stack(buf, true)
if n < len(buf) {
return buf[:n]
}
buf = make([]byte, 2*len(buf))
}
}
33 changes: 33 additions & 0 deletions service/debug_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package service

import (
"testing"
"time"

"github.com/safing/portmaster/base/notifications"
"github.com/safing/portmaster/service/mgr"
)

func TestDebug(t *testing.T) {
t.Parallel()

// Create test instance with at least one worker.
i := &Instance{}
n, err := notifications.New(i)
if err != nil {
t.Fatal(err)
}
i.serviceGroup = mgr.NewGroup(n)
i.SpnGroup = mgr.NewExtendedGroup()
err = i.Start()
if err != nil {
t.Fatal(err)
}
time.Sleep(100 * time.Millisecond)

info, err := i.GetWorkerInfo()
if err != nil {
t.Fatal(err)
}
t.Log(info)
}
2 changes: 1 addition & 1 deletion service/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ func (i *Instance) shutdown(exitCode int) {

// Stopping returns whether the instance is shutting down.
func (i *Instance) Stopping() bool {
return i.ctx.Err() == nil
return i.ctx.Err() != nil
}

// Stopped returns a channel that is triggered when the instance has shut down.
Expand Down
54 changes: 34 additions & 20 deletions service/mgr/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
package mgr

import (
"fmt"
"slices"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -107,36 +106,51 @@ func (em *EventMgr[T]) Submit(event T) {

// Run callbacks.
for _, ec := range em.callbacks {
// Check if callback was canceled.
if ec.canceled.Load() {
anyCanceled = true
continue
}

// Execute callback.
var (
cancel bool
err error
)
if em.mgr != nil {
// Prefer executing in worker.
wkrErr := em.mgr.Do("execute event callback", func(w *WorkerCtx) error {
cancel, err = ec.callback(w, event) //nolint:scopelint // Execution is within scope.
name := "event " + em.name + " callback " + ec.name
em.mgr.Go(name, func(w *WorkerCtx) error {
cancel, err = ec.callback(w, event)
// Handle error and cancelation.
if err != nil {
w.Warn(
"event callback failed",
"event", em.name,
"callback", ec.name,
"err", err,
)
}
if cancel {
ec.canceled.Store(true)
}
return nil
})
if wkrErr != nil {
err = fmt.Errorf("callback execution failed: %w", wkrErr)
}
} else {
cancel, err = ec.callback(nil, event)
}

// Handle error and cancelation.
if err != nil && em.mgr != nil {
em.mgr.Warn(
"event callback failed",
"event", em.name,
"callback", ec.name,
"err", err,
)
}
if cancel {
ec.canceled.Store(true)
anyCanceled = true
// Handle error and cancelation.
if err != nil && em.mgr != nil {
em.mgr.Warn(
"event callback failed",
"event", em.name,
"callback", ec.name,
"err", err,
)
}
if cancel {
ec.canceled.Store(true)
anyCanceled = true
}
}
}

Expand Down
1 change: 1 addition & 0 deletions service/mgr/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const (
groupStateInvalid
)

//nolint:goconst
func groupStateToString(state int32) string {
switch state {
case groupStateOff:
Expand Down
12 changes: 10 additions & 2 deletions service/mgr/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"log/slog"
"runtime"
"sync"
"sync/atomic"
"time"
)
Expand All @@ -21,6 +22,10 @@ type Manager struct {

workerCnt atomic.Int32
workersDone chan struct{}

workers []*WorkerCtx
workersIndex int
workersLock sync.Mutex
}

// New returns a new manager.
Expand All @@ -33,6 +38,7 @@ func newManager(name string) *Manager {
name: name,
logger: slog.Default().With(ManagerNameSLogKey, name),
workersDone: make(chan struct{}),
workers: make([]*WorkerCtx, 4),
}
m.ctx, m.cancelCtx = context.WithCancel(context.Background())
return m
Expand Down Expand Up @@ -196,11 +202,13 @@ func (m *Manager) waitForWorkers(max time.Duration, limit int32) (done bool) {
}
}

func (m *Manager) workerStart() {
func (m *Manager) workerStart(w *WorkerCtx) {
m.registerWorker(w)
m.workerCnt.Add(1)
}

func (m *Manager) workerDone() {
func (m *Manager) workerDone(w *WorkerCtx) {
m.unregisterWorker(w)
if m.workerCnt.Add(-1) <= 1 {
// Notify all waiters.
for {
Expand Down
6 changes: 3 additions & 3 deletions service/mgr/states.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,11 @@ func (m *StateMgr) Remove(id string) {
// Clear removes all states.
func (m *StateMgr) Clear() {
m.statesLock.Lock()
defer m.statesLock.Unlock()

m.states = nil
m.statesLock.Unlock()

m.statesEventMgr.Submit(m.export())
// Submit event without lock, because callbacks might come back to change states.
defer m.statesEventMgr.Submit(m.Export())
}

// Export returns the current states.
Expand Down
Loading

0 comments on commit 91f4fe5

Please sign in to comment.