Skip to content

Commit

Permalink
[fix] Panic while accessing SleepyTicker methods Stop()/SetSleep()
Browse files Browse the repository at this point in the history
The time.Ticker object was stored as a value type, but it is expected to be a pointer according to its implementation:
```
func (t *Ticker) Stop()
func (t *Ticker) Reset(d Duration)
```

This was leading to an application crash.

STR 1:
Run `portmaster-core` without privileged rights. It will not be able to start the kernel driver (Windows).
During unloading of already initialized modules, the process crashes because of stopping SleepyTicker instances in workers of the "network" module.

STR 2:
Run tests from `service\mgr\sleepyticker_test.go`
  • Loading branch information
stenya committed Dec 17, 2024
1 parent 6e173e3 commit 6c014d2
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 2 deletions.
4 changes: 2 additions & 2 deletions service/mgr/sleepyticker.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import "time"

// SleepyTicker is wrapper over time.Ticker that respects the sleep mode of the module.
type SleepyTicker struct {
ticker time.Ticker
ticker *time.Ticker
normalDuration time.Duration
sleepDuration time.Duration
sleepMode bool
Expand All @@ -16,7 +16,7 @@ type SleepyTicker struct {
// If sleepDuration is set to 0 ticker will not tick during sleep.
func NewSleepyTicker(normalDuration time.Duration, sleepDuration time.Duration) *SleepyTicker {
st := &SleepyTicker{
ticker: *time.NewTicker(normalDuration),
ticker: time.NewTicker(normalDuration),
normalDuration: normalDuration,
sleepDuration: sleepDuration,
sleepMode: false,
Expand Down
57 changes: 57 additions & 0 deletions service/mgr/sleepyticker_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package mgr

import (
"testing"
"time"
)

func TestSleepyTickerStop(t *testing.T) {

Check failure on line 8 in service/mgr/sleepyticker_test.go

View workflow job for this annotation

GitHub Actions / Linter

Function TestSleepyTickerStop missing the call to method parallel
normalDuration := 100 * time.Millisecond
sleepDuration := 200 * time.Millisecond

st := NewSleepyTicker(normalDuration, sleepDuration)
st.Stop() // no panic expected here
}

func TestSleepyTicker(t *testing.T) {

Check failure on line 16 in service/mgr/sleepyticker_test.go

View workflow job for this annotation

GitHub Actions / Linter

Function TestSleepyTicker missing the call to method parallel
normalDuration := 100 * time.Millisecond
sleepDuration := 200 * time.Millisecond

st := NewSleepyTicker(normalDuration, sleepDuration)

// Test normal mode
select {
case <-st.Wait():
// Expected tick
case <-time.After(normalDuration + 50*time.Millisecond):
t.Error("expected tick in normal mode")
}

// Test sleep mode
st.SetSleep(true)
select {
case <-st.Wait():
// Expected tick
case <-time.After(sleepDuration + 50*time.Millisecond):
t.Error("expected tick in sleep mode")
}

// Test sleep mode with sleepDuration == 0
st = NewSleepyTicker(normalDuration, 0)
st.SetSleep(true)
select {
case <-st.Wait():
t.Error("did not expect tick when sleepDuration is 0")
case <-time.After(normalDuration):
// Expected no tick
}

// Test stopping the ticker
st.Stop()
select {
case <-st.Wait():
t.Error("did not expect tick after stopping the ticker")
case <-time.After(normalDuration):
// Expected no tick
}
}

0 comments on commit 6c014d2

Please sign in to comment.