From e762378c42b786233ea13affa1cc2ee132ceefaf Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 12 Nov 2019 17:22:28 -0800 Subject: [PATCH] runtime: acquire timersLocks around moveTimers In the discussion of CL 171828 we decided that it was not necessary to acquire timersLock around the call to moveTimers, because the world is stopped. However, that is not correct, as sysmon runs even when the world is stopped, and it calls timeSleepUntil which looks through the timers. timeSleepUntil acquires timersLock, but that doesn't help if moveTimers is running at the same time. Updates #6239 Updates #27707 Updates #35462 Change-Id: I346c5bde594c4aff9955ae430b37c2b6fc71567f Reviewed-on: https://go-review.googlesource.com/c/go/+/206938 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Michael Knyszek --- src/runtime/proc.go | 9 ++++++++- src/runtime/time.go | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 3252173c0a2323..34d5928aa33662 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -4073,10 +4073,17 @@ func (pp *p) destroy() { } if len(pp.timers) > 0 { plocal := getg().m.p.ptr() - // The world is stopped so we don't need to hold timersLock. + // The world is stopped, but we acquire timersLock to + // protect against sysmon calling timeSleepUntil. + // This is the only case where we hold the timersLock of + // more than one P, so there are no deadlock concerns. + lock(&plocal.timersLock) + lock(&pp.timersLock) moveTimers(plocal, pp.timers) pp.timers = nil pp.adjustTimers = 0 + unlock(&pp.timersLock) + unlock(&plocal.timersLock) } // If there's a background worker, make it runnable and put // it on the global queue so it can clean itself up. diff --git a/src/runtime/time.go b/src/runtime/time.go index 39df413ad9db9d..47b326289bed2c 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -855,8 +855,8 @@ func cleantimers(pp *p) bool { // moveTimers moves a slice of timers to pp. The slice has been taken // from a different P. -// This is currently called when the world is stopped, but it could -// work as long as the timers for pp are locked. +// This is currently called when the world is stopped, but the caller +// is expected to have locked the timers for pp. func moveTimers(pp *p, timers []*timer) { for _, t := range timers { loop: