Fix a channel closing bug

siderolabs · Mar 27, 2017 · 428fa1c · 428fa1c
1 parent af55d61
commit 428fa1c
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 3 deletions.
diff --git a/proxy/handler.go b/proxy/handler.go
@@ -76,9 +76,10 @@ func (s *handler) handler(srv interface{}, serverStream grpc.ServerStream) error
 		return err
 	}
 	s2cErrChan := s.forwardServerToClient(serverStream, clientStream)
-	defer close(s2cErrChan)
 	c2sErrChan := s.forwardClientToServer(clientStream, serverStream)
-	defer close(c2sErrChan)
+	// Explicitly *do not close* s2cErrChan and c2sErrChan, otherwise the select below will not terminate.
+	// See https://groups.google.com/forum/#!msg/golang-nuts/pZwdYRGxCIk/qpbHxRRPJdUJ
+
 	// We don't know which side is going to stop sending first, so we need a select between the two.
 	for i := 0; i < 2; i++ {
 		select {

diff --git a/proxy/handler_test.go b/proxy/handler_test.go
@@ -121,6 +121,12 @@ func (s *ProxyHappySuite) TestPingEmptyCarriesClientMetadata() {
 	require.Equal(s.T(), &pb.PingResponse{Value: pingDefaultValue, Counter: 42}, out)
 }
 
+func (s *ProxyHappySuite) TestPingEmpty_StressTest() {
+	for i := 0; i < 50; i++ {
+		s.TestPingEmptyCarriesClientMetadata()
+	}
+}
+
 func (s *ProxyHappySuite) TestPingCarriesServerHeadersAndTrailers() {
 	headerMd := make(metadata.MD)
 	trailerMd := make(metadata.MD)
@@ -175,6 +181,12 @@ func (s *ProxyHappySuite) TestPingStream_FullDuplexWorks() {
 	assert.Len(s.T(), trailerMd, 1, "PingList trailer headers user contain metadata")
 }
 
+func (s *ProxyHappySuite) TestPingStream_StressTest() {
+	for i := 0; i < 50; i++ {
+		s.TestPingStream_FullDuplexWorks()
+	}
+}
+
 func (s *ProxyHappySuite) SetupSuite() {
 	var err error
 
@@ -224,7 +236,7 @@ func (s *ProxyHappySuite) SetupSuite() {
 	s.testClient = pb.NewTestServiceClient(clientConn)
 }
 
-func (s *ProxyHappySuite) TearDownSuite() {
+func (s *ProxyHappySuite) xTearDownSuite() {
 	if s.proxy != nil {
 		s.proxy.Stop()
 		s.proxyListener.Close()