Ensure that calling grpcServer.Stop() only once

[hunter007]

Fix #411

[codecov]

Codecov Report

Merging #412 (ae4c514) into main (df5bd08) will increase coverage by 0.39%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #412      +/-   ##
==========================================
+ Coverage   69.97%   70.37%   +0.39%     
==========================================
  Files          33       33              
  Lines        2651     2653       +2     
==========================================
+ Hits         1855     1867      +12     
+ Misses        695      686       -9     
+ Partials      101      100       -1     

Impacted Files	Coverage Δ
service/grpc/service.go	77.96% <100.00%> (+18.31%)	⬆️

[daixiang0]

If not started, double call will return nil. Is it an issue that we miss update the start status?

[hunter007]

Yes. I will correct it

[daixiang0]

Also, we can use start status to check stop, no need to add a new one.

[hunter007]

We can prevent restart and re-stop using stopped.

If no stopped, when started = 0, we can call Start() which set started=1.

But, call Stop(), if set started = 0, we can restart server? if keep started = 1, we can re-stop?

[daixiang0]

When started is:

• true, can stop, start/re-start but do nothing
• false, can start, stop/re-stop but do nothing

Only real stop/start will update started.

[hunter007]

I write a test:

func TestReStopServer(t *testing.T) {
	server := getTestServer()
	startTestServer(server)
	time.Sleep(time.Second)

	assert.NotNil(t, server)
	err := server.Stop()
	assert.Nilf(t, err, "error stopping server")

	fmt.Print("Re-stop server\n")

	err = server.Stop()
	assert.Nilf(t, err, "error re-stopping server")
}

the test panic:

go test -timeout 30s -run ^TestReStopServer$ github.com/dapr/go-sdk/service/grpc -v

=== RUN   TestReStopServer
Re-stop server
--- FAIL: TestReStopServer (1.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x180 pc=0x10475103c]

goroutine 34 [running]:
testing.tRunner.func1.2({0x1048f8860, 0x104d0c280})
	/go1.20/src/testing/testing.go:1526 +0x1c8
testing.tRunner.func1()
	/go1.20/src/testing/testing.go:1529 +0x364
panic({0x1048f8860, 0x104d0c280})
	/go1.20/src/runtime/panic.go:884 +0x1f4
google.golang.org/grpc.(*Server).Stop(0x0)
	/gopkg/mod/google.golang.org/grpc@v1.55.0/server.go:1800 +0x2c
github.com/dapr/go-sdk/service/grpc.(*Server).Stop(...)
	/dapr_repos/go-sdk/service/grpc/service.go:112
github.com/dapr/go-sdk/service/grpc.TestReStopServer(0x0?)
	/dapr_repos/go-sdk/service/grpc/service_test.go:79 +0x1c4
testing.tRunner(0x140001171e0, 0x10499b2f0)
	/go1.20/src/testing/testing.go:1576 +0x104
created by testing.(*T).Run
	/go1.20/src/testing/testing.go:1629 +0x370

I made this PR to resolve this issue: #411

[mikeee]

I think we should be using the require package for error evaluations e.g. require.Error / require.NoError