GracefulStop sometimes kills alive requests

[g7r]

What version of gRPC are you using?

v1.21.1

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

go version go1.12.6 linux/amd64

What operating system (Linux, Windows, …) and version?

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Pop!_OS 19.04
Release:	19.04
Codename:	disco

What did you do?

package main

import (
	"context"
	"fmt"
	"net"
	"runtime"
	"testing"

	"google.golang.org/grpc"
	"google.golang.org/grpc/test/grpc_testing"
)

type testServer struct {
	grpc_testing.TestServiceServer
	emptyCallFn func(ctx context.Context, in *grpc_testing.Empty) (*grpc_testing.Empty, error)
}

func (s *testServer) EmptyCall(ctx context.Context, in *grpc_testing.Empty) (*grpc_testing.Empty, error) {
	return s.emptyCallFn(ctx, in)
}

func TestGracefulStop(t *testing.T) {
	for i := 0; i < 2000; i++ {
		t.Run(fmt.Sprintf("N%d", i), func(t *testing.T) {
			doTest(t, i)
		})
	}
}

func doTest(t *testing.T, nYield int) {
	var ts testServer
	server := grpc.NewServer()
	defer server.Stop()
	grpc_testing.RegisterTestServiceServer(server, &ts)
	listener, err := net.Listen("tcp", "127.0.0.1:")
	if err != nil {
		t.Fatalf("net.Listen failed: %v", err)
	}

	serverDone := make(chan struct{})
	go func() {
		defer close(serverDone)
		if err := server.Serve(listener); err != nil {
			t.Errorf("server.Serve failed: %v", err)
		}
	}()

	clientConn, err := grpc.Dial(fmt.Sprintf("127.0.0.1:%d", listener.Addr().(*net.TCPAddr).Port), grpc.WithBlock(), grpc.WithInsecure())
	if err != nil {
		t.Fatalf("grpc.Dial failed: %v", err)
	}
	defer clientConn.Close()

	emptyCallFnInvoked := make(chan struct{})
	ts.emptyCallFn = func(context.Context, *grpc_testing.Empty) (*grpc_testing.Empty, error) {
		close(emptyCallFnInvoked)
		for i := 0; i < nYield; i++ {
			runtime.Gosched()
		}
		return &grpc_testing.Empty{}, nil
	}

	clientDone := make(chan struct{})
	go func() {
		defer close(clientDone)
		client := grpc_testing.NewTestServiceClient(clientConn)
		if _, err := client.EmptyCall(context.Background(), &grpc_testing.Empty{}); err != nil {
			t.Fatalf("client.EmptyCall failed: %v", err)
		}
	}()

	<-emptyCallFnInvoked
	server.GracefulStop()

	<-clientDone
	<-serverDone
}

This test tries to gracefully shutdown server while request is being handled. To consistently reproduce the error I have to use loop. This issue reproduces on v1.21.1 and not reproduces on v1.18.0. I've bisected to find the culprit and it is likely to be 32559e2.

What did you expect to see?

Tests shouldn't fail.

What did you see instead?

Tests fail flakily. I have ~3% fail rate on my laptop.

[dfawley]

Could you please try testing this with #2857 and see if your problems are resolved? If so, we could do a 1.21.2 patch release that includes this fix, or we will be releasing 1.22 tomorrow if you are OK with waiting.

[g7r]

#2857 fixes my issue.
I'm OK with 1.22.