From 978cefcaf82b8a51180f5fa792527e832adf26f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aitor=20P=C3=A9rez=20Cedres?= Date: Fri, 30 Sep 2022 13:10:29 +0100 Subject: [PATCH 1/2] Fix example client to avoid deadlock in Close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The example was "giving up" on publish confirmation after the resendDelay. This is not correct because the library will keep track of this confirmation, and it will try to deliver the confirmation. By giving up, we are may leave confirmations un-received in the confirmation channel, which will cause a deadlock during the shut down sequence in Channel.Close. We should not give up on the confirmation and simply wait. Signed-off-by: Aitor Pérez Cedres --- example_client_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/example_client_test.go b/example_client_test.go index 63034cf..c473978 100644 --- a/example_client_test.go +++ b/example_client_test.go @@ -68,7 +68,7 @@ func Example_consume() { } // This channel will receive a notification when a channel closed event - // happens. This must be different than Client.notifyChanClose because the + // happens. This must be different from Client.notifyChanClose because the // library sends only one notification and Client.notifyChanClose already has // a receiver in handleReconnect(). // Recommended to make it buffered to avoid deadlocks @@ -273,8 +273,6 @@ func (client *Client) changeChannel(channel *amqp.Channel) { } // Push will push data onto the queue, and wait for a confirm. -// If no confirms are received until within the resendTimeout, -// it continuously re-sends messages until a confirm is received. // This will block until the server sends a confirm. Errors are // only returned if the push action itself fails, see UnsafePush. func (client *Client) Push(data []byte) error { @@ -295,12 +293,10 @@ func (client *Client) Push(data []byte) error { select { case confirm := <-client.notifyConfirm: if confirm.Ack { - client.logger.Println("Push confirmed!") + client.logger.Printf("Push confirmed [%d]!", confirm.DeliveryTag) return nil } - case <-time.After(resendDelay): } - client.logger.Println("Push didn't confirm. Retrying...") } } @@ -357,7 +353,7 @@ func (client *Client) Consume() (<-chan amqp.Delivery, error) { ) } -// Close will cleanly shutdown the channel and connection. +// Close will cleanly shut down the channel and connection. func (client *Client) Close() error { if !client.isReady { return errAlreadyClosed From 280ec661d480b5064813d4c7d7df2b9b891e1344 Mon Sep 17 00:00:00 2001 From: Luke Bakken Date: Sat, 1 Oct 2022 09:36:43 -0700 Subject: [PATCH 2/2] Fix golangci-lint error --- example_client_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/example_client_test.go b/example_client_test.go index c473978..2c8d6d7 100644 --- a/example_client_test.go +++ b/example_client_test.go @@ -290,12 +290,10 @@ func (client *Client) Push(data []byte) error { } continue } - select { - case confirm := <-client.notifyConfirm: - if confirm.Ack { - client.logger.Printf("Push confirmed [%d]!", confirm.DeliveryTag) - return nil - } + confirm := <-client.notifyConfirm + if confirm.Ack { + client.logger.Printf("Push confirmed [%d]!", confirm.DeliveryTag) + return nil } } }