Skip to content

Commit

Permalink
Wait for OpenSSL server shutdown in e2e test
Browse files Browse the repository at this point in the history
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. We do so by moving from
implicit shutdown via context cancellation to explicit shutdown.

https://tip.golang.org/doc/go1.20

Signed-off-by: Daniel Mangum <georgedanielmangum@gmail.com>
  • Loading branch information
hasheddan committed Jun 30, 2023
1 parent 159122f commit d7303d0
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
8 changes: 6 additions & 2 deletions e2e/e2e_openssl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func serverOpenSSL(c *comm) {

// launch command
// #nosec G204
cmd := exec.CommandContext(c.ctx, "openssl", args...)
cmd := exec.Command("openssl", args...)
var inner net.Conn
inner, c.serverConn = net.Pipe()
cmd.Stdin = inner
Expand All @@ -95,6 +95,8 @@ func serverOpenSSL(c *comm) {

c.serverReady <- struct{}{}
simpleReadWrite(c.errChan, c.serverChan, c.serverConn, c.messageRecvCount)
c.serverDone <- cmd.Process.Kill()
close(c.serverDone)
}()
}

Expand Down Expand Up @@ -155,7 +157,7 @@ func clientOpenSSL(c *comm) {

// launch command
// #nosec G204
cmd := exec.CommandContext(c.ctx, "openssl", args...)
cmd := exec.Command("openssl", args...)
var inner net.Conn
inner, c.clientConn = net.Pipe()
cmd.Stdin = inner
Expand All @@ -168,6 +170,8 @@ func clientOpenSSL(c *comm) {
}

simpleReadWrite(c.errChan, c.clientChan, c.clientConn, c.messageRecvCount)
c.clientDone <- cmd.Process.Kill()
close(c.clientDone)
}

func ciphersOpenSSL(cfg *dtls.Config) string {
Expand Down
41 changes: 41 additions & 0 deletions e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,12 @@ type comm struct {
messageRecvCount *uint64 // Counter to make sure both sides got a message
clientMutex *sync.Mutex
clientConn net.Conn
clientDone chan error
serverMutex *sync.Mutex
serverConn net.Conn
serverListener net.Listener
serverReady chan struct{}
serverDone chan error
errChan chan error
clientChan chan string
serverChan chan string
Expand All @@ -107,6 +109,8 @@ func newComm(ctx context.Context, clientConfig, serverConfig *dtls.Config, serve
clientMutex: &sync.Mutex{},
serverMutex: &sync.Mutex{},
serverReady: make(chan struct{}),
serverDone: make(chan error),
clientDone: make(chan error),
errChan: make(chan error),
clientChan: make(chan string),
serverChan: make(chan string),
Expand Down Expand Up @@ -172,6 +176,32 @@ func (c *comm) assert(t *testing.T) {
}()
}

func (c *comm) cleanup(t *testing.T) {
clientDone, serverDone := false, false
for {
select {
case err := <-c.clientDone:
if err != nil {
t.Fatal(err)
}
clientDone = true
if clientDone && serverDone {
return
}
case err := <-c.serverDone:
if err != nil {
t.Fatal(err)
}
serverDone = true
if clientDone && serverDone {
return
}
case <-time.After(testTimeLimit):
t.Fatalf("Test timeout waiting for server shutdown")
}
}
}

func clientPion(c *comm) {
select {
case <-c.serverReady:
Expand All @@ -194,6 +224,8 @@ func clientPion(c *comm) {
}

simpleReadWrite(c.errChan, c.clientChan, c.clientConn, c.messageRecvCount)
c.clientDone <- nil
close(c.clientDone)
}

func serverPion(c *comm) {
Expand All @@ -217,6 +249,8 @@ func serverPion(c *comm) {
}

simpleReadWrite(c.errChan, c.serverChan, c.serverConn, c.messageRecvCount)
c.serverDone <- nil
close(c.serverDone)
}

/*
Expand Down Expand Up @@ -254,6 +288,7 @@ func testPionE2ESimple(t *testing.T, server, client func(*comm)) {
}
serverPort := randomPort(t)
comm := newComm(ctx, cfg, cfg, serverPort, server, client)
defer comm.cleanup(t)
comm.assert(t)
})
}
Expand Down Expand Up @@ -287,6 +322,7 @@ func testPionE2ESimplePSK(t *testing.T, server, client func(*comm)) {
}
serverPort := randomPort(t)
comm := newComm(ctx, cfg, cfg, serverPort, server, client)
defer comm.cleanup(t)
comm.assert(t)
})
}
Expand Down Expand Up @@ -322,6 +358,7 @@ func testPionE2EMTUs(t *testing.T, server, client func(*comm)) {
}
serverPort := randomPort(t)
comm := newComm(ctx, cfg, cfg, serverPort, server, client)
defer comm.cleanup(t)
comm.assert(t)
})
}
Expand Down Expand Up @@ -362,6 +399,7 @@ func testPionE2ESimpleED25519(t *testing.T, server, client func(*comm)) {
}
serverPort := randomPort(t)
comm := newComm(ctx, cfg, cfg, serverPort, server, client)
defer comm.cleanup(t)
comm.assert(t)
})
}
Expand Down Expand Up @@ -407,6 +445,7 @@ func testPionE2ESimpleED25519ClientCert(t *testing.T, server, client func(*comm)
}
serverPort := randomPort(t)
comm := newComm(ctx, ccfg, scfg, serverPort, server, client)
defer comm.cleanup(t)
comm.assert(t)
}

Expand Down Expand Up @@ -450,6 +489,7 @@ func testPionE2ESimpleECDSAClientCert(t *testing.T, server, client func(*comm))
}
serverPort := randomPort(t)
comm := newComm(ctx, ccfg, scfg, serverPort, server, client)
defer comm.cleanup(t)
comm.assert(t)
}

Expand Down Expand Up @@ -493,6 +533,7 @@ func testPionE2ESimpleRSAClientCert(t *testing.T, server, client func(*comm)) {
}
serverPort := randomPort(t)
comm := newComm(ctx, ccfg, scfg, serverPort, server, client)
defer comm.cleanup(t)
comm.assert(t)
}

Expand Down

0 comments on commit d7303d0

Please sign in to comment.