diff --git a/pkg/sentry/fs/timerfd/timerfd.go b/pkg/sentry/fs/timerfd/timerfd.go index ca8be8683..457227954 100644 --- a/pkg/sentry/fs/timerfd/timerfd.go +++ b/pkg/sentry/fs/timerfd/timerfd.go @@ -141,12 +141,9 @@ func (t *TimerOperations) Write(context.Context, *fs.File, usermem.IOSequence, i return 0, linuxerr.EINVAL } -// Notify implements ktime.TimerListener.Notify. -func (t *TimerOperations) Notify(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { +// NotifyTimer implements ktime.TimerListener.NotifyTimer. +func (t *TimerOperations) NotifyTimer(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { atomic.AddUint64(&t.val, exp) t.events.Notify(waiter.ReadableEvents) return ktime.Setting{}, false } - -// Destroy implements ktime.TimerListener.Destroy. -func (t *TimerOperations) Destroy() {} diff --git a/pkg/sentry/fsimpl/timerfd/timerfd.go b/pkg/sentry/fsimpl/timerfd/timerfd.go index 68b785791..565dfa6a0 100644 --- a/pkg/sentry/fsimpl/timerfd/timerfd.go +++ b/pkg/sentry/fsimpl/timerfd/timerfd.go @@ -47,7 +47,7 @@ type TimerFileDescription struct { } var _ vfs.FileDescriptionImpl = (*TimerFileDescription)(nil) -var _ ktime.TimerListener = (*TimerFileDescription)(nil) +var _ ktime.Listener = (*TimerFileDescription)(nil) // New returns a new timer fd. func New(ctx context.Context, vfsObj *vfs.VirtualFilesystem, clock ktime.Clock, flags uint32) (*vfs.FileDescription, error) { @@ -136,12 +136,9 @@ func (tfd *TimerFileDescription) Release(context.Context) { tfd.timer.Destroy() } -// Notify implements ktime.TimerListener.Notify. -func (tfd *TimerFileDescription) Notify(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { +// NotifyTimer implements ktime.TimerListener.NotifyTimer. +func (tfd *TimerFileDescription) NotifyTimer(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { atomic.AddUint64(&tfd.val, exp) tfd.events.Notify(waiter.ReadableEvents) return ktime.Setting{}, false } - -// Destroy implements ktime.TimerListener.Destroy. -func (tfd *TimerFileDescription) Destroy() {} diff --git a/pkg/sentry/kernel/posixtimer.go b/pkg/sentry/kernel/posixtimer.go index 319754a42..07205cc68 100644 --- a/pkg/sentry/kernel/posixtimer.go +++ b/pkg/sentry/kernel/posixtimer.go @@ -115,8 +115,8 @@ func (it *IntervalTimer) signalRejectedLocked() { it.overrunCur++ } -// Notify implements ktime.TimerListener.Notify. -func (it *IntervalTimer) Notify(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { +// NotifyTimer implements ktime.TimerListener.NotifyTimer. +func (it *IntervalTimer) NotifyTimer(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { if it.target == nil { return ktime.Setting{}, false } @@ -151,11 +151,6 @@ func (it *IntervalTimer) Notify(exp uint64, setting ktime.Setting) (ktime.Settin return ktime.Setting{}, false } -// Destroy implements ktime.TimerListener.Destroy. Users of Timer should call -// DestroyTimer instead. -func (it *IntervalTimer) Destroy() { -} - // IntervalTimerCreate implements timer_create(2). func (t *Task) IntervalTimerCreate(c ktime.Clock, sigev *linux.Sigevent) (linux.TimerID, error) { t.tg.timerMu.Lock() diff --git a/pkg/sentry/kernel/task_sched.go b/pkg/sentry/kernel/task_sched.go index 9d9fa76a6..9882f1c12 100644 --- a/pkg/sentry/kernel/task_sched.go +++ b/pkg/sentry/kernel/task_sched.go @@ -352,8 +352,8 @@ func newKernelCPUClockTicker(k *Kernel) *kernelCPUClockTicker { } } -// Notify implements ktime.TimerListener.Notify. -func (ticker *kernelCPUClockTicker) Notify(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { +// NotifyTimer implements ktime.TimerListener.NotifyTimer. +func (ticker *kernelCPUClockTicker) NotifyTimer(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { // Only increment cpuClock by 1 regardless of the number of expirations. // This approximately compensates for cases where thread throttling or bad // Go runtime scheduling prevents the kernelCPUClockTicker goroutine, and @@ -472,10 +472,6 @@ func (ticker *kernelCPUClockTicker) Notify(exp uint64, setting ktime.Setting) (k return setting, false } -// Destroy implements ktime.TimerListener.Destroy. -func (ticker *kernelCPUClockTicker) Destroy() { -} - // randInt31n returns a random integer in [0, n). // // randInt31n is equivalent to math/rand.Rand.int31n(), which is unexported. diff --git a/pkg/sentry/kernel/thread_group.go b/pkg/sentry/kernel/thread_group.go index 5814a4eca..64659d2ac 100644 --- a/pkg/sentry/kernel/thread_group.go +++ b/pkg/sentry/kernel/thread_group.go @@ -532,12 +532,8 @@ type itimerRealListener struct { tg *ThreadGroup } -// Notify implements ktime.TimerListener.Notify. -func (l *itimerRealListener) Notify(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { +// NotifyTimer implements ktime.TimerListener.NotifyTimer. +func (l *itimerRealListener) NotifyTimer(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { l.tg.SendSignal(SignalInfoPriv(linux.SIGALRM)) return ktime.Setting{}, false } - -// Destroy implements ktime.TimerListener.Destroy. -func (l *itimerRealListener) Destroy() { -} diff --git a/pkg/sentry/kernel/time/BUILD b/pkg/sentry/kernel/time/BUILD index e293d9a0f..59fbcd9d6 100644 --- a/pkg/sentry/kernel/time/BUILD +++ b/pkg/sentry/kernel/time/BUILD @@ -6,8 +6,8 @@ go_library( name = "time", srcs = [ "context.go", - "tcpip.go", "time.go", + "util.go", ], visibility = ["//pkg/sentry:internal"], deps = [ diff --git a/pkg/sentry/kernel/time/time.go b/pkg/sentry/kernel/time/time.go index 191b92811..e7652282a 100644 --- a/pkg/sentry/kernel/time/time.go +++ b/pkg/sentry/kernel/time/time.go @@ -277,10 +277,10 @@ func (*ClockEventsQueue) Readiness(mask waiter.EventMask) waiter.EventMask { return 0 } -// A TimerListener receives expirations from a Timer. -type TimerListener interface { - // Notify is called when its associated Timer expires. exp is the number of - // expirations. setting is the next timer Setting. +// Listener receives expirations from a Timer. +type Listener interface { + // NotifyTimer is called when its associated Timer expires. exp is the number + // of expirations. setting is the next timer Setting. // // Notify is called with the associated Timer's mutex locked, so Notify // must not take any locks that precede Timer.mu in lock order. @@ -289,10 +289,7 @@ type TimerListener interface { // rather than the passed one. // // Preconditions: exp > 0. - Notify(exp uint64, setting Setting) (newSetting Setting, update bool) - - // Destroy is called when the timer is destroyed. - Destroy() + NotifyTimer(exp uint64, setting Setting) (newSetting Setting, update bool) } // Setting contains user-controlled mutable Timer properties. @@ -415,7 +412,7 @@ type Timer struct { clock Clock // listener is notified of expirations. listener is immutable. - listener TimerListener + listener Listener // mu protects the following mutable fields. mu sync.Mutex `state:"nosave"` @@ -449,7 +446,7 @@ const timerTickEvents = ClockEventSet | ClockEventRateIncrease // NewTimer returns a new Timer that will obtain time from clock and send // expirations to listener. The Timer is initially stopped and has no first // expiration or period configured. -func NewTimer(clock Clock, listener TimerListener) *Timer { +func NewTimer(clock Clock, listener Listener) *Timer { t := &Timer{ clock: clock, listener: listener, @@ -488,7 +485,6 @@ func (t *Timer) Destroy() { // before closing t.events to instruct the Timer goroutine to exit. t.clock.EventUnregister(&t.entry) close(t.events) - t.listener.Destroy() } func (t *Timer) runGoroutine() { @@ -517,7 +513,7 @@ func (t *Timer) Tick() { s, exp := t.setting.At(now) t.setting = s if exp > 0 { - if newS, ok := t.listener.Notify(exp, t.setting); ok { + if newS, ok := t.listener.NotifyTimer(exp, t.setting); ok { t.setting = newS } } @@ -574,7 +570,7 @@ func (t *Timer) Get() (Time, Setting) { s, exp := t.setting.At(now) t.setting = s if exp > 0 { - if newS, ok := t.listener.Notify(exp, t.setting); ok { + if newS, ok := t.listener.NotifyTimer(exp, t.setting); ok { t.setting = newS } } @@ -610,7 +606,7 @@ func (t *Timer) SwapAnd(s Setting, f func()) (Time, Setting) { } oldS, oldExp := t.setting.At(now) if oldExp > 0 { - t.listener.Notify(oldExp, oldS) + t.listener.NotifyTimer(oldExp, oldS) // N.B. The returned Setting doesn't matter because we're about // to overwrite. } @@ -620,7 +616,7 @@ func (t *Timer) SwapAnd(s Setting, f func()) (Time, Setting) { newS, newExp := s.At(now) t.setting = newS if newExp > 0 { - if newS, ok := t.listener.Notify(newExp, t.setting); ok { + if newS, ok := t.listener.NotifyTimer(newExp, t.setting); ok { t.setting = newS } } @@ -658,35 +654,26 @@ func (t *Timer) Clock() Clock { return t.clock } -// ChannelNotifier is a TimerListener that sends a message on an empty struct -// channel. +// ChannelNotifier is a Listener that sends on a channel. // // ChannelNotifier cannot be saved or loaded. -type ChannelNotifier struct { - // tchan must be a buffered channel. - tchan chan struct{} -} +type ChannelNotifier chan struct{} // NewChannelNotifier creates a new channel notifier. // // If the notifier is used with a timer, Timer.Destroy will close the channel // returned here. -func NewChannelNotifier() (TimerListener, <-chan struct{}) { +func NewChannelNotifier() (Listener, <-chan struct{}) { tchan := make(chan struct{}, 1) - return &ChannelNotifier{tchan}, tchan + return ChannelNotifier(tchan), tchan } -// Notify implements ktime.TimerListener.Notify. -func (c *ChannelNotifier) Notify(uint64, Setting) (Setting, bool) { +// NotifyTimer implements Listener.NotifyTimer. +func (c ChannelNotifier) NotifyTimer(uint64, Setting) (Setting, bool) { select { - case c.tchan <- struct{}{}: + case c <- struct{}{}: default: } return Setting{}, false } - -// Destroy implements ktime.TimerListener.Destroy and will close the channel. -func (c *ChannelNotifier) Destroy() { - close(c.tchan) -} diff --git a/pkg/sentry/kernel/time/tcpip.go b/pkg/sentry/kernel/time/util.go similarity index 79% rename from pkg/sentry/kernel/time/tcpip.go rename to pkg/sentry/kernel/time/util.go index c4474c0cf..3e487235f 100644 --- a/pkg/sentry/kernel/time/tcpip.go +++ b/pkg/sentry/kernel/time/util.go @@ -19,32 +19,32 @@ import ( "time" ) -// TcpipAfterFunc waits for duration to elapse according to clock then runs fn. +// AfterFunc waits for duration to elapse according to clock then runs fn. // The timer is started immediately and will fire exactly once. -func TcpipAfterFunc(clock Clock, duration time.Duration, fn func()) *TcpipTimer { - timer := &TcpipTimer{ +func AfterFunc(clock Clock, duration time.Duration, fn func()) *VariableTimer { + timer := &VariableTimer{ clock: clock, } timer.notifier = functionNotifier{ fn: func() { // tcpip.Timer.Stop() explicitly states that the function is called in a // separate goroutine that Stop() does not synchronize with. - // Timer.Destroy() synchronizes with calls to TimerListener.Notify(). + // Timer.Destroy() synchronizes with calls to Listener.NotifyTimer(). // This is semantically meaningful because, in the former case, it's // legal to call tcpip.Timer.Stop() while holding locks that may also be // taken by the function, but this isn't so in the latter case. Most - // immediately, Timer calls TimerListener.Notify() while holding + // immediately, Timer calls Listener.NotifyTimer() while holding // Timer.mu. A deadlock occurs without spawning a goroutine: // T1: (Timer expires) // => Timer.Tick() <- Timer.mu.Lock() called - // => TimerListener.Notify() + // => Listener.NotifyTimer() // => Timer.Stop() // => Timer.Destroy() <- Timer.mu.Lock() called, deadlock! // // Spawning a goroutine avoids the deadlock: // T1: (Timer expires) // => Timer.Tick() <- Timer.mu.Lock() called - // => TimerListener.Notify() <- Launches T2 + // => Listener.NotifyTimer() <- Launches T2 // T2: // => Timer.Stop() // => Timer.Destroy() <- Timer.mu.Lock() called, blocks @@ -62,12 +62,12 @@ func TcpipAfterFunc(clock Clock, duration time.Duration, fn func()) *TcpipTimer return timer } -// TcpipTimer is a resettable timer with variable duration expirations. +// VariableTimer is a resettable timer with variable duration expirations. // Implements tcpip.Timer, which does not define a Destroy method; instead, all // resources are released after timer expiration and calls to Timer.Stop. // // Must be created by AfterFunc. -type TcpipTimer struct { +type VariableTimer struct { // clock is the time source. clock is immutable. clock Clock @@ -85,7 +85,7 @@ type TcpipTimer struct { } // Stop implements tcpip.Timer.Stop. -func (r *TcpipTimer) Stop() bool { +func (r *VariableTimer) Stop() bool { r.mu.Lock() defer r.mu.Unlock() @@ -99,7 +99,7 @@ func (r *TcpipTimer) Stop() bool { } // Reset implements tcpip.Timer.Reset. -func (r *TcpipTimer) Reset(d time.Duration) { +func (r *VariableTimer) Reset(d time.Duration) { r.mu.Lock() defer r.mu.Unlock() @@ -121,11 +121,8 @@ type functionNotifier struct { fn func() } -// Notify implements ktime.TimerListener.Notify. -func (f *functionNotifier) Notify(uint64, Setting) (Setting, bool) { +// NotifyTimer implements ktime.TimerListener.NotifyTimer. +func (f *functionNotifier) NotifyTimer(uint64, Setting) (Setting, bool) { f.fn() return Setting{}, false } - -// Destroy implements ktime.TimerListener.Destroy. -func (f *functionNotifier) Destroy() {} diff --git a/pkg/sentry/kernel/timekeeper.go b/pkg/sentry/kernel/timekeeper.go index 6255bae7a..a68e495f3 100644 --- a/pkg/sentry/kernel/timekeeper.go +++ b/pkg/sentry/kernel/timekeeper.go @@ -200,7 +200,7 @@ func (t *Timekeeper) NowMonotonic() tcpip.MonotonicTime { // AfterFunc implements tcpip.Clock. func (t *Timekeeper) AfterFunc(d time.Duration, f func()) tcpip.Timer { - return ktime.TcpipAfterFunc(t.realtimeClock, d, f) + return ktime.AfterFunc(t.realtimeClock, d, f) } // startUpdater starts an update goroutine that keeps the clocks updated.