From 7254d2c9d75a30fe034918752ab04e94f973b12d Mon Sep 17 00:00:00 2001 From: Paulo Neves Date: Fri, 11 Feb 2022 15:19:20 +0100 Subject: [PATCH 1/3] client: set disconnect as defer This makes it easy to refactor the conditional and makes sure that it always gets executed. --- client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client.go b/client.go index 06ee61a..e7b33eb 100644 --- a/client.go +++ b/client.go @@ -452,6 +452,8 @@ func (c *client) attemptConnection() (net.Conn, byte, bool, error) { // reusing the `client` may lead to panics. If you want to reconnect when the connection drops then use // `SetAutoReconnect` and/or `SetConnectRetry`options instead of implementing this yourself. func (c *client) Disconnect(quiesce uint) { + defer c.disconnect() + status := atomic.LoadUint32(&c.status) if status == connected { DEBUG.Println(CLI, "disconnecting") @@ -480,7 +482,6 @@ func (c *client) Disconnect(quiesce uint) { c.setConnected(disconnected) } - c.disconnect() } // forceDisconnect will end the connection with the mqtt broker immediately (used for tests only) From 3ec7db339d21308d4baaa028e9e2cce2759877d6 Mon Sep 17 00:00:00 2001 From: Paulo Neves Date: Fri, 11 Feb 2022 15:21:26 +0100 Subject: [PATCH 2/3] client: refactor setConnected(disconnected) out of conditional c.setConnected(disconnected) was called inconditionally in both conditions start so it is in fact independent of the status. Move it out. --- client.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client.go b/client.go index e7b33eb..84b9b88 100644 --- a/client.go +++ b/client.go @@ -455,9 +455,9 @@ func (c *client) Disconnect(quiesce uint) { defer c.disconnect() status := atomic.LoadUint32(&c.status) + c.setConnected(disconnected) if status == connected { DEBUG.Println(CLI, "disconnecting") - c.setConnected(disconnected) dm := packets.NewControlPacket(packets.Disconnect).(*packets.DisconnectPacket) dt := newToken(packets.Disconnect) @@ -479,7 +479,6 @@ func (c *client) Disconnect(quiesce uint) { } } else { WARN.Println(CLI, "Disconnect() called but not connected (disconnected/reconnecting)") - c.setConnected(disconnected) } } From ac29fdf5dbd9840c81414431062955c4ae3d1cef Mon Sep 17 00:00:00 2001 From: Paulo Neves Date: Fri, 11 Feb 2022 15:29:05 +0100 Subject: [PATCH 3/3] client: localized disconnectSent logic the diconnectSent logic was being used in it's own conditional when it naturally is not conditional when the writing to c.ouboundP is successful. Signed-off-by: Paulo Neves --- client.go | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/client.go b/client.go index 84b9b88..4c869f1 100644 --- a/client.go +++ b/client.go @@ -456,31 +456,26 @@ func (c *client) Disconnect(quiesce uint) { status := atomic.LoadUint32(&c.status) c.setConnected(disconnected) - if status == connected { - DEBUG.Println(CLI, "disconnecting") - dm := packets.NewControlPacket(packets.Disconnect).(*packets.DisconnectPacket) - dt := newToken(packets.Disconnect) - disconnectSent := false - select { - case c.oboundP <- &PacketAndToken{p: dm, t: dt}: - disconnectSent = true - case <-c.commsStopped: - WARN.Println("Disconnect packet could not be sent because comms stopped") - case <-time.After(time.Duration(quiesce) * time.Millisecond): - WARN.Println("Disconnect packet not sent due to timeout") - } - - // wait for work to finish, or quiesce time consumed - if disconnectSent { - DEBUG.Println(CLI, "calling WaitTimeout") - dt.WaitTimeout(time.Duration(quiesce) * time.Millisecond) - DEBUG.Println(CLI, "WaitTimeout done") - } - } else { + if status != connected { WARN.Println(CLI, "Disconnect() called but not connected (disconnected/reconnecting)") + return } + DEBUG.Println(CLI, "disconnecting") + dm := packets.NewControlPacket(packets.Disconnect).(*packets.DisconnectPacket) + dt := newToken(packets.Disconnect) + select { + case c.oboundP <- &PacketAndToken{p: dm, t: dt}: + // wait for work to finish, or quiesce time consumed + DEBUG.Println(CLI, "calling WaitTimeout") + dt.WaitTimeout(time.Duration(quiesce) * time.Millisecond) + DEBUG.Println(CLI, "WaitTimeout done") + case <-c.commsStopped: + WARN.Println("Disconnect packet could not be sent because comms stopped") + case <-time.After(time.Duration(quiesce) * time.Millisecond): + WARN.Println("Disconnect packet not sent due to timeout") + } } // forceDisconnect will end the connection with the mqtt broker immediately (used for tests only)