Skip to content

Commit a11eb1b

Browse files
fraenkelBryan C. Mills
authored andcommitted
http2: wait for both client and server funcs
When executing the clienttester.run(), it would exit when both the client and server completed successfully or if either side failed. If one side fails, the other side may not have finished. If an error was reported, the goroutine was caught executing after the test completed. The expectation is always that both sides complete successfully. In the case where one side fails, we still need to wait for the other side to complete. Close both client and server connections on error to expedite the completion. Fixes golang/go#41299 Change-Id: If47fbe61de42495bb2b1327bd5b03d6c295670dc Reviewed-on: https://go-review.googlesource.com/c/net/+/267760 Reviewed-by: Bryan C. Mills <[email protected]> Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Dmitri Shuralyov <[email protected]>
1 parent ff519b6 commit a11eb1b

File tree

1 file changed

+20
-24
lines changed

1 file changed

+20
-24
lines changed

http2/transport_test.go

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -757,36 +757,32 @@ func (ct *clientTester) readNonSettingsFrame() (Frame, error) {
757757

758758
func (ct *clientTester) cleanup() {
759759
ct.tr.CloseIdleConnections()
760+
761+
// close both connections, ignore the error if its already closed
762+
ct.sc.Close()
763+
ct.cc.Close()
760764
}
761765

762766
func (ct *clientTester) run() {
763-
errc := make(chan error, 2)
764-
ct.start("client", errc, ct.client)
765-
ct.start("server", errc, ct.server)
766-
defer ct.cleanup()
767-
for i := 0; i < 2; i++ {
768-
if err := <-errc; err != nil {
769-
ct.t.Error(err)
770-
return
767+
var errOnce sync.Once
768+
var wg sync.WaitGroup
769+
770+
run := func(which string, fn func() error) {
771+
defer wg.Done()
772+
if err := fn(); err != nil {
773+
errOnce.Do(func() {
774+
ct.t.Errorf("%s: %v", which, err)
775+
ct.cleanup()
776+
})
771777
}
772778
}
773-
}
774779

775-
func (ct *clientTester) start(which string, errc chan<- error, fn func() error) {
776-
go func() {
777-
finished := false
778-
var err error
779-
defer func() {
780-
if !finished {
781-
err = fmt.Errorf("%s goroutine didn't finish.", which)
782-
} else if err != nil {
783-
err = fmt.Errorf("%s: %v", which, err)
784-
}
785-
errc <- err
786-
}()
787-
err = fn()
788-
finished = true
789-
}()
780+
wg.Add(2)
781+
go run("client", ct.client)
782+
go run("server", ct.server)
783+
wg.Wait()
784+
785+
errOnce.Do(ct.cleanup) // clean up if no error
790786
}
791787

792788
func (ct *clientTester) readFrame() (Frame, error) {

0 commit comments

Comments
 (0)