runtime: preemption in startTemplateThread may cause infinite hang #38931

prattmic · 2020-05-07T20:22:44Z

Summary:

If a call to startTemplateThread is preempted by a gcstop before creating the new thread, then newmHandoff looks initialized even though it is not. When other locked Ms stop they may handoff their (now _Pidle) P to a new M. Creation of that new M will never occur because the template thread is not running, and stopTheWorldWithSema will hang forever waiting for the P to enter _Pgcstop.

Details:

When a locked M wants to start a new M, it hands off to the template thread to actually call clone and start the thread. The template thread is lazily created the first time a thread is locked (or if cgo is in use).

stoplockedm will release the P (_Pidle), then call handoffp to give the P to another M. In the case of a pending STW, one of two things can happen:

handoffp starts an M, which does acquirep followed by schedule, which will finally enter _Pgcstop.
handoffp immediately enters _Pgcstop. This only occurs if the P has no local work, GC work, and no spinning M is required.

If handoffp starts an M, and must create a new M to do so, then newm simply queue the M on newmHandoff for the template thread to do the clone.

If the template thread was never fully created because startTemplateThread is stopped, then that thread will never get created, and the handoff P will never enter _Pgcstop.

To visualize, consider two threads running when a STW occurs:

  T1                                        T2
--------------------------------         -----------------------------

LockOSThread                             LockOSThread
  haveTemplateThread == 0
  startTemplateThread
    haveTemplateThread = 1
    newm                                   haveTemplateThread == 1
      preempt -> schedule                  g.m.lockedExt++
        gcstopm -> _Pgcstop                g.m.lockedg = ...
        park                               g.lockedm = ...
                                           return

                                        ... (any code)
                                          preempt -> schedule
                                            stoplockedm
                                              releasep -> _Pidle
                                              handoffp
                                                startm (one of first three handoffp cases)
                                                 newm
                                                   g.m.lockedExt != 0
                                                   Add to newmHandoff, return
                                              park

Note that T2's P is sitting in _Pidle. It expects the template thread to clone the new M, which would then acquirep, schedule, and gcstopm. Since the template thread doesn't exist yet, this never happens, and stopTheWorldWithSema waits forever for the last P to enter stop.

I believe the best fix for this is to disable preemption in startTemplateThread (acquirem) so we can guarantee that the template thread will eventually exist after haveTemplateThread = 1.

I've reproduced this issue with an internal program, and the suggested acquirem fixes that case. This should be reproducible with something like the below program, but I haven't quite managed to get that to trigger the bug:

package main

import (
        "runtime"
        "sync"
        "time"
)

//go:noinline
func work() int {
        // Do something complicated on the stack so this function gets a morestack call.

        var arr [10000]int
        for i := 0; i < 10000; i++ {
                arr[i] = i
        }

        var sum int
        for i := 0; i < 10000; i++ {
                sum += arr[i]
        }

        return sum
}

func main() {
        // Try to synchronize STW and LockOSThreads.
        start := time.Now().Add(50*time.Millisecond)

        go func() {
                // The goroutines below are a bit slower. Add a fudge factor.
                for time.Now().Before(start.Add(10*time.Microsecond)) {
                }

                // Stop the world.
                var m runtime.MemStats
                runtime.ReadMemStats(&m)
        }()

        var wg sync.WaitGroup
        wg.Add(2)

        for i := 0; i < 2; i++ {
                go func() {
                        for time.Now().Before(start) {
                        }

                        // Add work to the local runq to trigger early startm
                        // in handoffp.
                        go func(){}()

                        runtime.LockOSThread()
                        work()  // add a preemption point.
                        wg.Done()
                }()
        }

        wg.Wait()
        // If both LockOSThreads completed then we did not hit the race.
}

cc @aclements

The text was updated successfully, but these errors were encountered:

prattmic · 2020-05-07T20:26:28Z

@gopherbot backport please 1.13 1.14

gopherbot · 2020-05-07T20:26:37Z

Backport issue(s) opened: #38932 (for 1.13), #38933 (for 1.14).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

prattmic · 2020-05-07T21:29:00Z

Doh, it turns out reproducer programs don't work well if you build them with the fix in the runtime.

Without the fix, the above repro hangs in ~1/50 runs, right where expected:

Thread 4 (LWP 156917):
#0  runtime.futex () at /usr/local/google/home/mpratt/src/go/src/runtime/sys_linux_amd64.s:568
#1  0x00000000004279b4 in runtime.futexsleep (addr=0x4f08a8 <runtime.sched+264>, val=0, ns=100000) at /usr/local/google/home/mpratt/src/go/src/runtime/os_linux.go:51
#2  0x0000000000408b6e in runtime.notetsleep_internal (n=0x4f08a8 <runtime.sched+264>, ns=100000, ~r2=<optimized out>) at /usr/local/google/home/mpratt/src/go/src/runtime/lock_futex.go:193
#3  0x0000000000408c41 in runtime.notetsleep (n=0x4f08a8 <runtime.sched+264>, ns=100000, ~r2=<optimized out>) at /usr/local/google/home/mpratt/src/go/src/runtime/lock_futex.go:216
#4  0x000000000042fa15 in runtime.stopTheWorldWithSema () at /usr/local/google/home/mpratt/src/go/src/runtime/proc.go:943
#5  0x00000000004536a6 in runtime.systemstack () at /usr/local/google/home/mpratt/src/go/src/runtime/asm_amd64.s:370
#6  0x000000000042fce0 in ?? () at <autogenerated>:1
#7  0x0000000000000000 in ?? ()

gopherbot · 2020-05-08T15:01:43Z

Change https://golang.org/cl/232978 mentions this issue: runtime: disable preemption in startTemplateThread

prattmic · 2020-05-08T19:08:48Z

N.B. the test in https://golang.org/cl/232978 reproduces this issue in 1.14.

It does not reproduce on 1.13, but I believe the issue should still exist there, the repro timing may just be a bit off.

gopherbot · 2020-05-21T21:06:59Z

Change https://golang.org/cl/234885 mentions this issue: [release-branch.go1.14] runtime: disable preemption in startTemplateThread

gopherbot · 2020-05-21T21:13:55Z

Change https://golang.org/cl/234888 mentions this issue: [release-branch.go1.13] runtime: disable preemption in startTemplateThread

…hread When a locked M wants to start a new M, it hands off to the template thread to actually call clone and start the thread. The template thread is lazily created the first time a thread is locked (or if cgo is in use). stoplockedm will release the P (_Pidle), then call handoffp to give the P to another M. In the case of a pending STW, one of two things can happen: 1. handoffp starts an M, which does acquirep followed by schedule, which will finally enter _Pgcstop. 2. handoffp immediately enters _Pgcstop. This only occurs if the P has no local work, GC work, and no spinning M is required. If handoffp starts an M, and must create a new M to do so, then newm will simply queue the M on newmHandoff for the template thread to do the clone. When a stop-the-world is required, stopTheWorldWithSema will start the stop and then wait for all Ps to enter _Pgcstop. If the template thread is not fully created because startTemplateThread gets stopped, then another stoplockedm may queue an M that will never get created, and the handoff P will never leave _Pidle. Thus stopTheWorldWithSema will wait forever. A sequence to trigger this hang when STW occurs can be visualized with two threads: T1 T2 ------------------------------- ----------------------------- LockOSThread LockOSThread haveTemplateThread == 0 startTemplateThread haveTemplateThread = 1 newm haveTemplateThread == 1 preempt -> schedule g.m.lockedExt++ gcstopm -> _Pgcstop g.m.lockedg = ... park g.lockedm = ... return ... (any code) preempt -> schedule stoplockedm releasep -> _Pidle handoffp startm (first 3 handoffp cases) newm g.m.lockedExt != 0 Add to newmHandoff, return park Note that the P in T2 is stuck sitting in _Pidle. Since the template thread isn't running, the new M will not be started complete the transition to _Pgcstop. To resolve this, we disable preemption around the assignment of haveTemplateThread and the creation of the template thread in order to guarantee that if handTemplateThread is set then the template thread will eventually exist, in the presence of stops. For #38931 Fixes #38933 Change-Id: I50535fbbe2f328f47b18e24d9030136719274191 Reviewed-on: https://go-review.googlesource.com/c/go/+/232978 Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> (cherry picked from commit 11b3730) Reviewed-on: https://go-review.googlesource.com/c/go/+/234885 Reviewed-by: Cherry Zhang <cherryyz@google.com>

…hread When a locked M wants to start a new M, it hands off to the template thread to actually call clone and start the thread. The template thread is lazily created the first time a thread is locked (or if cgo is in use). stoplockedm will release the P (_Pidle), then call handoffp to give the P to another M. In the case of a pending STW, one of two things can happen: 1. handoffp starts an M, which does acquirep followed by schedule, which will finally enter _Pgcstop. 2. handoffp immediately enters _Pgcstop. This only occurs if the P has no local work, GC work, and no spinning M is required. If handoffp starts an M, and must create a new M to do so, then newm will simply queue the M on newmHandoff for the template thread to do the clone. When a stop-the-world is required, stopTheWorldWithSema will start the stop and then wait for all Ps to enter _Pgcstop. If the template thread is not fully created because startTemplateThread gets stopped, then another stoplockedm may queue an M that will never get created, and the handoff P will never leave _Pidle. Thus stopTheWorldWithSema will wait forever. A sequence to trigger this hang when STW occurs can be visualized with two threads: T1 T2 ------------------------------- ----------------------------- LockOSThread LockOSThread haveTemplateThread == 0 startTemplateThread haveTemplateThread = 1 newm haveTemplateThread == 1 preempt -> schedule g.m.lockedExt++ gcstopm -> _Pgcstop g.m.lockedg = ... park g.lockedm = ... return ... (any code) preempt -> schedule stoplockedm releasep -> _Pidle handoffp startm (first 3 handoffp cases) newm g.m.lockedExt != 0 Add to newmHandoff, return park Note that the P in T2 is stuck sitting in _Pidle. Since the template thread isn't running, the new M will not be started complete the transition to _Pgcstop. To resolve this, we disable preemption around the assignment of haveTemplateThread and the creation of the template thread in order to guarantee that if handTemplateThread is set then the template thread will eventually exist, in the presence of stops. For #38931 Fixes #38932 Change-Id: I50535fbbe2f328f47b18e24d9030136719274191 Reviewed-on: https://go-review.googlesource.com/c/go/+/232978 Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> (cherry picked from commit 11b3730) Reviewed-on: https://go-review.googlesource.com/c/go/+/234888 Reviewed-by: Cherry Zhang <cherryyz@google.com>

This fixes the following issue in Go runtime, which could result in program hang - golang/go#38931

prattmic self-assigned this May 7, 2020

This was referenced May 7, 2020

runtime: preemption in startTemplateThread may cause infinite hang [1.13 backport] #38932

Closed

runtime: preemption in startTemplateThread may cause infinite hang [1.14 backport] #38933

Closed

prattmic added the NeedsFix The path to resolution is known, but the work has not been done. label May 7, 2020

prattmic added this to the Go1.15 milestone May 7, 2020

gopherbot closed this as completed in 11b3730 May 21, 2020

valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this issue Jun 4, 2020

deployment/docker: update Go builder from v1.14.3 to v1.14.4

af49c5b

This fixes the following issue in Go runtime, which could result in program hang - golang/go#38931

valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this issue Jun 4, 2020

deployment/docker: update Go builder from v1.14.3 to v1.14.4

d49f059

This fixes the following issue in Go runtime, which could result in program hang - golang/go#38931

golang locked and limited conversation to collaborators May 21, 2021

gopherbot added the FrozenDueToAge label May 21, 2021

rsc unassigned prattmic Jun 23, 2022

prattmic self-assigned this Jun 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime: preemption in startTemplateThread may cause infinite hang #38931

runtime: preemption in startTemplateThread may cause infinite hang #38931

prattmic commented May 7, 2020 •

edited

prattmic commented May 7, 2020

gopherbot commented May 7, 2020

prattmic commented May 7, 2020

gopherbot commented May 8, 2020

prattmic commented May 8, 2020

gopherbot commented May 21, 2020

gopherbot commented May 21, 2020

runtime: preemption in startTemplateThread may cause infinite hang #38931

runtime: preemption in startTemplateThread may cause infinite hang #38931

Comments

prattmic commented May 7, 2020 • edited

prattmic commented May 7, 2020

gopherbot commented May 7, 2020

prattmic commented May 7, 2020

gopherbot commented May 8, 2020

prattmic commented May 8, 2020

gopherbot commented May 21, 2020

gopherbot commented May 21, 2020

prattmic commented May 7, 2020 •

edited