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 1/2] =?UTF-8?q?fix:=20issue=20675=EF=BC=9Agoroutine=20leak?= =?UTF-8?q?=20when=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 af5538e..a46cde5 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) From 71f981419c22a5af18598e180dd31345d70eb1a8 Mon Sep 17 00:00:00 2001 From: kiqi007 <457965634@qq.com> Date: Mon, 20 May 2024 09:31:44 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20issue=20675=EF=BC=9Agoroutine=20leak?= =?UTF-8?q?=20when=20connectionUp(true)=20return=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- client.go | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/client.go b/client.go index a46cde5..b88d932 100644 --- a/client.go +++ b/client.go @@ -601,26 +601,12 @@ func (c *client) startCommsWorkers(conn net.Conn, connectionUp connCompletedFn, c.workers.Add(1) // Done will be called when ackOut is closed ackOut := c.msgRouter.matchAndDispatch(incomingPubChan, c.options.Order, c) - // 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... + // 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... + // issue 675:we will allow the connection to complete before the Disconnect is allowed to proceed + // as if a Disconnect event occurred immediately after connectionUp(true) completed. if err := connectionUp(true); err != nil { 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")