fix: wait for OpenSSL server shutdown in e2e test

Updates e2e tests to wait until server shutdown before completing to ensure that we do not trigger errors due to leaked watchCtx goroutine, which was added to os/exec.Command in Go 1.20. https://tip.golang.org/doc/go1.20 Signed-off-by: Daniel Mangum <georgedanielmangum@gmail.com>
pion · Jun 29, 2023 · d245060 · d245060
1 parent 8a11cf2
commit d245060
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 64 deletions.
diff --git a/e2e/Dockerfile b/e2e/Dockerfile
@@ -1,7 +1,7 @@
 # SPDX-FileCopyrightText: 2023 The Pion community <https://pion.ly>
 # SPDX-License-Identifier: MIT
 
-FROM docker.io/library/golang:1.18-bullseye
+FROM docker.io/library/golang:1.20-bullseye
 
 COPY . /go/src/github.com/pion/dtls
 WORKDIR /go/src/github.com/pion/dtls/e2e

diff --git a/e2e/e2e_openssl_test.go b/e2e/e2e_openssl_test.go
@@ -24,78 +24,78 @@ import (
 )
 
 func serverOpenSSL(c *comm) {
-	go func() {
-		c.serverMutex.Lock()
-		defer c.serverMutex.Unlock()
-
-		cfg := c.serverConfig
-
-		// create openssl arguments
-		args := []string{
-			"s_server",
-			"-dtls1_2",
-			"-quiet",
-			"-verify_quiet",
-			"-verify_return_error",
-			fmt.Sprintf("-accept=%d", c.serverPort),
-		}
-		ciphers := ciphersOpenSSL(cfg)
-		if ciphers != "" {
-			args = append(args, fmt.Sprintf("-cipher=%s", ciphers))
-		}
+	c.serverMutex.Lock()
+	defer c.serverMutex.Unlock()
 
-		// psk arguments
-		if cfg.PSK != nil {
-			psk, err := cfg.PSK(nil)
-			if err != nil {
-				c.errChan <- err
-				return
-			}
-			args = append(args, fmt.Sprintf("-psk=%X", psk))
-			if len(cfg.PSKIdentityHint) > 0 {
-				args = append(args, fmt.Sprintf("-psk_hint=%s", cfg.PSKIdentityHint))
-			}
-		}
+	cfg := c.serverConfig
 
-		// certs arguments
-		if len(cfg.Certificates) > 0 {
-			// create temporary cert files
-			certPEM, keyPEM, err := writeTempPEM(cfg)
-			if err != nil {
-				c.errChan <- err
-				return
-			}
-			args = append(args,
-				fmt.Sprintf("-cert=%s", certPEM),
-				fmt.Sprintf("-key=%s", keyPEM))
-			defer func() {
-				_ = os.Remove(certPEM)
-				_ = os.Remove(keyPEM)
-			}()
-		} else {
-			args = append(args, "-nocert")
+	// create openssl arguments
+	args := []string{
+		"s_server",
+		"-dtls1_2",
+		"-quiet",
+		"-verify_quiet",
+		"-verify_return_error",
+		fmt.Sprintf("-accept=%d", c.serverPort),
+	}
+	ciphers := ciphersOpenSSL(cfg)
+	if ciphers != "" {
+		args = append(args, fmt.Sprintf("-cipher=%s", ciphers))
+	}
+
+	// psk arguments
+	if cfg.PSK != nil {
+		psk, err := cfg.PSK(nil)
+		if err != nil {
+			c.errChan <- err
+			return
 		}
+		args = append(args, fmt.Sprintf("-psk=%X", psk))
+		if len(cfg.PSKIdentityHint) > 0 {
+			args = append(args, fmt.Sprintf("-psk_hint=%s", cfg.PSKIdentityHint))
+		}
+	}
 
-		// launch command
-		// #nosec G204
-		cmd := exec.CommandContext(c.ctx, "openssl", args...)
-		var inner net.Conn
-		inner, c.serverConn = net.Pipe()
-		cmd.Stdin = inner
-		cmd.Stdout = inner
-		cmd.Stderr = os.Stderr
-		if err := cmd.Start(); err != nil {
+	// certs arguments
+	if len(cfg.Certificates) > 0 {
+		// create temporary cert files
+		certPEM, keyPEM, err := writeTempPEM(cfg)
+		if err != nil {
 			c.errChan <- err
-			_ = inner.Close()
 			return
 		}
+		args = append(args,
+			fmt.Sprintf("-cert=%s", certPEM),
+			fmt.Sprintf("-key=%s", keyPEM))
+		defer func() {
+			_ = os.Remove(certPEM)
+			_ = os.Remove(keyPEM)
+		}()
+	} else {
+		args = append(args, "-nocert")
+	}
+
+	// launch command
+	// #nosec G204
+	cmd := exec.CommandContext(c.ctx, "openssl", args...)
+	var inner net.Conn
+	inner, c.serverConn = net.Pipe()
+	cmd.Stdin = inner
+	cmd.Stdout = inner
+	cmd.Stderr = os.Stderr
+	if err := cmd.Start(); err != nil {
+		c.errChan <- err
+		_ = inner.Close()
+		return
+	}
 
-		// Ensure that server has started
-		time.Sleep(500 * time.Millisecond)
+	// Ensure that server has started
+	time.Sleep(500 * time.Millisecond)
 
-		c.serverReady <- struct{}{}
-		simpleReadWrite(c.errChan, c.serverChan, c.serverConn, c.messageRecvCount)
-	}()
+	c.serverReady <- struct{}{}
+	simpleReadWrite(c.errChan, c.serverChan, c.serverConn, c.messageRecvCount)
+	c.serverDone <- cmd.Wait()
+	close(c.serverDone)
 }
 
 func clientOpenSSL(c *comm) {

diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go
@@ -89,6 +89,7 @@ type comm struct {
 	serverConn                 net.Conn
 	serverListener             net.Listener
 	serverReady                chan struct{}
+	serverDone                 chan error
 	errChan                    chan error
 	clientChan                 chan string
 	serverChan                 chan string
@@ -107,6 +108,7 @@ func newComm(ctx context.Context, clientConfig, serverConfig *dtls.Config, serve
 		clientMutex:      &sync.Mutex{},
 		serverMutex:      &sync.Mutex{},
 		serverReady:      make(chan struct{}),
+		serverDone:       make(chan error),
 		errChan:          make(chan error),
 		clientChan:       make(chan string),
 		serverChan:       make(chan string),
@@ -139,6 +141,14 @@ func (c *comm) assert(t *testing.T) {
 				t.Fatal(err)
 			}
 		}
+		select {
+		case err := <-c.serverDone:
+			if err != nil {
+				t.Fatal(err)
+			}
+		case <-time.After(testTimeLimit):
+			t.Fatalf("Test timeout waiting for server shutdown")
+		}
 	}()
 
 	func() {
@@ -217,6 +227,8 @@ func serverPion(c *comm) {
 	}
 
 	simpleReadWrite(c.errChan, c.serverChan, c.serverConn, c.messageRecvCount)
+	c.serverDone <- nil
+	close(c.serverDone)
 }
 
 /*