Skip to content

Commit

Permalink
resolved easwar's comments: refactored server code in test and comments
Browse files Browse the repository at this point in the history
  • Loading branch information
aranjans committed Jul 15, 2024
1 parent 89c7299 commit 35ff394
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 32 deletions.
4 changes: 2 additions & 2 deletions internal/transport/http2_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,7 @@ func (t *http2Client) closeStream(s *Stream, err error, rst bool, rstCode http2.
}

// Close kicks off the shutdown process of the transport. This should be called
// only once on transport. Once it is called, the transport should not be
// only once on a transport. Once it is called, the transport should not be
// accessed anymore.
func (t *http2Client) Close(err error) {
t.mu.Lock()
Expand Down Expand Up @@ -1027,7 +1027,7 @@ func (t *http2Client) Close(err error) {
st = status.New(codes.Unavailable, err.Error())
}
case <-time.After(goAwayLoopyWriterTimeout):
t.logger.Warningf("Failed to write a GOAWAY frame as part of connection close after %v. Giving up and closing the transport.", goAwayLoopyWriterTimeout)
t.logger.Warningf("Failed to write a GOAWAY frame as part of connection close after %s. Giving up and closing the transport.", goAwayLoopyWriterTimeout)
}
t.cancel()
t.conn.Close()
Expand Down
41 changes: 11 additions & 30 deletions internal/transport/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2752,10 +2752,9 @@ func (s) TestClientSendsAGoAwayFrame(t *testing.T) {
// LoopyWriterTimeout, client should still be close itself and should
// not wait for long.
func (s) TestClientCloseReturnsEarlyWhenGoAwayWriteHangs(t *testing.T) {
// Override goAwayLoopyWriterTimeout to 0 so that we always
// time out while writing GOAWAY on client.Close(). This is
// equivalent to network hang scenario when client is
// failing to write GOAWAY frame.
// Override timer for writing GOAWAY to 0 so that the connection write
// always times out. It is equivalent of real network hang when conn
// write for goaway doesn't finish in specified deadline
origGoAwayLoopyTimeout := goAwayLoopyWriterTimeout
goAwayLoopyWriterTimeout = 0
defer func() {
Expand All @@ -2768,42 +2767,25 @@ func (s) TestClientCloseReturnsEarlyWhenGoAwayWriteHangs(t *testing.T) {
t.Fatalf("Error while listening: %v", err)
}
defer lis.Close()
// serverGreetingDone is used to notify when server is done greeting the client.
serverGreetingDone := make(chan struct{})
// errorCh verifies that desired GOAWAY not received by server
errorCh := make(chan error, 1)
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
// Launch the server.
// Launch the server and handle HTTP/2 connections, specifically
// focusing on the reception and interpretation of GOAWAY frames
// sent from client.
go func() {
defer close(errorCh)
conn, err := lis.Accept()
if err != nil {
t.Errorf("Error while accepting: %v", err)
}
defer conn.Close()
if _, err := io.ReadFull(conn, make([]byte, len(clientPreface))); err != nil {
t.Errorf("Error while reading client preface: %v", err)
return
}
framer := http2.NewFramer(conn, conn)
if err := framer.WriteSettings(); err != nil {
t.Errorf("Error while writing settings %v", err)
return
}
fr, _ := framer.ReadFrame()
if _, ok := fr.(*http2.SettingsFrame); !ok {
t.Errorf("Expected settings frame, got %T", fr)
}
fr, _ = framer.ReadFrame()
if fr, ok := fr.(*http2.SettingsFrame); !ok || !fr.IsAck() {
t.Errorf("Expected settings ACK frame, got %T", fr)
}
fr, _ = framer.ReadFrame()
if fr, ok := fr.(*http2.HeadersFrame); !ok || !fr.Flags.Has(http2.FlagHeadersEndHeaders) {
t.Errorf("Expected Headers frame with END_HEADERS frame, got %T", fr)
}
close(serverGreetingDone)

frame, err := framer.ReadFrame()
if err != nil {
Expand All @@ -2830,16 +2812,15 @@ func (s) TestClientCloseReturnsEarlyWhenGoAwayWriteHangs(t *testing.T) {
if err != nil {
t.Fatalf("failed to open stream: %v", err)
}
// Wait until server receives the headers and settings frame as part of greet.
<-serverGreetingDone
// ct.Close will try to send the GOAWAY to server and will fail writing
// GOAWAY and will eventually close the loopyWriter as
// goAwayLoopyWriterTimeout (time out for writing GOAWAY) is set to zero, which is
// equivalent to network hang scenario.
// GOAWAY in case of network hang and will eventually close the loopyWriter as
// goAwayLoopyWriterTimeout (time out for writing GOAWAY) is set to 0, which is
// equivalent to network hang scenario when conn write doesn't finish
// within specified deadline.
ct.Close(errors.New("manually closed by client"))
select {
case <-errorCh:
case <-ctx.Done():
t.Errorf("Context timed out")
t.Errorf("timeout waiting for client Close(): Context timed out")
}
}

0 comments on commit 35ff394

Please sign in to comment.