x/sync: singleflight behaves incorrectly after Forget

[polomsky]

What version of Go are you using (go version)?

go version go1.12.1 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

not relevant

What did you do?

I am launching 3 tasks together with forget.

1. I started first task
2. during its run I called Forget and started second task, everything is all right, two tasks are running together as specified
3. after first task is finished, I am launching third task, please note that second task is still running, so execution of third task should be prevented

Situation in timeline:

task 1  111111111111
forget        F
task 2         2222222222222222222
task 3                  33333333333333...

func TestForget(t *testing.T) {
	var g Group

	var firstStarted, firstFinished sync.WaitGroup

	firstStarted.Add(1)
	firstFinished.Add(1)

	go func() {
		g.Do("key", func() (i interface{}, e error) {
			firstStarted.Done()
			time.Sleep(10 * time.Millisecond)
			firstFinished.Done()
			return
		})
	}()

	firstStarted.Wait()
	g.Forget("key")
	// from this point no two function using same key should be executed concurrently

	var secondStarted int32 = 0
	var secondFinished int32 = 0
	var thirdStarted int32 = 0

	go func() {
		g.Do("key", func() (i interface{}, e error) {
			atomic.AddInt32(&secondStarted, 1)
			time.Sleep(20 * time.Millisecond) // it should be longer than first call
			atomic.AddInt32(&secondFinished, 1)
			return 2, nil
		})
	}()

	firstFinished.Wait() // wait for first execution (which should not affect execution after Forget)

	if atomic.LoadInt32(&secondStarted) != 1 {
		t.Fatal("Second execution should be executed due to usage of forget")
	}

	if atomic.LoadInt32(&secondFinished) == 1 {
		t.Fatal("Second execution should be still active")
	}

	result, _, _ := g.Do("key", func() (i interface{}, e error) {
		atomic.AddInt32(&thirdStarted, 1)
		return 3, nil
	})

	if thirdStarted != 0 {
		t.Error("Third call should not be started because was started during second execution")
	}
	if result != 2 {
		t.Errorf("We should receive result produced by second call, expected: 2, got %d", result)
	}
}

What did you expect to see?

I expected that third task will not be executed

What did you see instead?

Third task has been executed. Exit of first task together with Forget disrupted protection of second task.

[bcmills]

CC @bradfitz

[gopherbot]

Change https://golang.org/cl/171897 mentions this issue: singleflight: fix duplicate deleting key when Forget called