Connection/Channel Deadlock about amqp091-go

[melardev]

The two danger zones:

• https://github.com/rabbitmq/amqp091-go/blob/main/connection.go#L413
• https://github.com/rabbitmq/amqp091-go/blob/main/channel.go#L104

I replaced both with a select statement to see if it runs great, I assumed it would break something else, I don't know if it did, but at least it does not hang anymore ....

select {
  case <-time.After(time.Second * 5):
    break
  case c <- err:
    break
  }

[melardev]

I think that the issue is the channel that is notified about channel close is no longer being used to catch the amqpError in my code, at that time the channel has been initiated ... and so notifying a channel that is not listening in Golang hangs ... very hard problem in Golang, I am not sure if you would ever fix this, or tag it as working as expected, but well ... IMO frameworks should never hang forever even if they are misused.

[Zerpet]

Thank you for reporting this issue. Deadlocks are something worth looking into. I'll keep this issue around to have a look at some point in the coming days.

[andygrunwald]

This issue is very close to #18.
Both describe the same behavior.

[remster]

Up-voting - burnt 2hrs yesterday on this. Might give client ability to unsubscribe, but best relieve them and avoid the deadlock by non-blocking write as suggested above.

[melardev]

@remster You can not unsubscribe, as soon as you call NotifyClose, the framework will take that chan, save it in its internal slice, and notify it when the RabbitMQ channel is closed, no matter what, you better have a listener on that chan otherwise the framework will be signaling an "orphan" chan leading to a deadlock.

[malc0mn]

Also running into this one and it really is the same as #18.
As already suggested in #18, I would also make the library notification code non blocking by using the idiomatic non blocking select:

select {
case c <- err:
default:
}

I think the amqp091-go library should not deadlock at all in this case. Not causing a deadlock is definitely preferred over caring if the user of the library receives proper alerts on shutdown, that really should be the users care but the documentation/examples should explain that.

If the idea is that the user of the library must properly subscribe to the connClose and chanClose notifications, then we should somehow implement these checks in the library so that the user will be made aware of this expectation.
But not deadlocking still seems preferable to me.

Edit: beginning to get a better understanding of the issue. With this new insight, what I said above is not the right solution. The examples should be better on how to handle things. I'll make sure to post some code when I'm done testing.

[DanielePalaia]

As for comment in #18, I'll cose this one as the issue seems the same.

[DanielePalaia]

As for comment in #18, I'll cose this one as the issue seems the same.