From 386b73136159a81d48113b2de1c81f2b225ac438 Mon Sep 17 00:00:00 2001
From: kiqi007 <457965634@qq.com>
Date: Sun, 19 May 2024 23:46:13 +0800
Subject: [PATCH] =?UTF-8?q?fix:=20issue=20675=EF=BC=9Agoroutine=20leak=20w?=
 =?UTF-8?q?hen=20connectionUp(true)=20return=20error?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 client.go | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/client.go b/client.go
index af5538e6..a46cde58 100644
--- a/client.go
+++ b/client.go
@@ -604,14 +604,25 @@ func (c *client) startCommsWorkers(conn net.Conn, connectionUp connCompletedFn,
 	// The connection is now ready for use (we spin up a few go routines below). It is possible that
 	// Disconnect has been called in the interim...
 	if err := connectionUp(true); err != nil {
-		DEBUG.Println(CLI, err)
-		close(c.stop) // Tidy up anything we have already started
-		close(incomingPubChan)
-		c.workers.Wait()
-		c.conn.Close()
-		c.conn = nil
-		return false
+		ERROR.Println(CLI, err)
+
+		/* issue 675：goroutine leak when connectionUp(true) return error
+		 *   Only when status == disconnecting will this logic be executed.
+		 * The goroutine that changes the status to disconnecting should be
+		 * responsible for resource cleanup (which is indeed how it is done).
+		 *
+		 *   Being disconnected right when the connection is established is a special case.
+		 * Even if we remove this check for connectionUp(true), the program must still function correctly,
+		 * as if a Disconnect event occurred immediately after connectionUp(true) completed.
+		 */
+
+		//close(c.stop) // Tidy up anything we have already started
+		//close(incomingPubChan)
+		//c.workers.Wait()
+		//c.conn.Close()
+		//c.conn = nil
 	}
+
 	DEBUG.Println(CLI, "client is connected/reconnected")
 	if c.options.OnConnect != nil {
 		go c.options.OnConnect(c)