From 46f880bc52b328ba8baba05334cfe430486ddbd8 Mon Sep 17 00:00:00 2001
From: Ivan Kozlovic <ivan@synadia.com>
Date: Fri, 8 May 2020 10:55:27 -0600
Subject: [PATCH] [FIXED] Early closed connection may linger in the server

If the connection is marked as closed while sending the INFO, the
connection would not be removed from the internal map, which would
cause it to be shown in the monitoring list of opened connections.

Resolves #1384

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
---
 server/client_test.go | 31 +++++++++++++++++++++++++++++++
 server/server.go      |  6 ++++++
 2 files changed, 37 insertions(+)

diff --git a/server/client_test.go b/server/client_test.go
index a05745fb..5379cd59 100644
--- a/server/client_test.go
+++ b/server/client_test.go
@@ -2038,3 +2038,34 @@ func TestCloseConnectionLogsReason(t *testing.T) {
 		t.Fatal("Log does not contain closed reason")
 	}
 }
+
+func TestCloseConnectionVeryEarly(t *testing.T) {
+	o := DefaultOptions()
+	s := RunServer(o)
+	defer s.Shutdown()
+
+	// The issue was with a connection that would break right when
+	// server was sending the INFO. Creating a bare TCP connection
+	// and closing it right away won't help reproduce the problem.
+	// So testing in 2 steps.
+
+	// Get a normal TCP connection to the server.
+	c, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", o.Port))
+	if err != nil {
+		s.mu.Unlock()
+		t.Fatalf("Unable to create tcp connection")
+	}
+	// Now close it.
+	c.Close()
+
+	// Wait that num clients falls to 0.
+	checkClientsCount(t, s, 0)
+
+	// Call again with this closed connection. Alternatively, we
+	// would have to call with a fake connection that implements
+	// net.Conn but returns an error on Write.
+	s.createClient(c)
+
+	// This connection should not have been added to the server.
+	checkClientsCount(t, s, 0)
+}
diff --git a/server/server.go b/server/server.go
index c85dce5e..f9c68b1f 100644
--- a/server/server.go
+++ b/server/server.go
@@ -1793,6 +1793,12 @@ func (s *Server) createClient(conn net.Conn) *client {
 	// The connection may have been closed
 	if c.isClosed() {
 		c.mu.Unlock()
+		// If it was due to TLS timeout, teardownConn() has already been called.
+		// Otherwise, if connection was marked as closed while sending the INFO,
+		// we need to call teardownConn() directly here.
+		if !info.TLSRequired {
+			c.teardownConn()
+		}
 		return c
 	}