Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix message sender shutdown order #328

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

annatisch
Copy link
Member

@annatisch annatisch commented Jan 15, 2020

The current messagesender_close function starts by clearing any pending messages by calling on_message_send_complete with an error state, and then attempts to detach the link.

However, if the link detach should fail for any reason (say, the connection is in an error state), then the link will also attempt to remove it's pending deliveries (on_session_state_change -> remove_all_pending_deliveries) which also calls the on_message_send_complete callback a second time.

This has the potential to fail in two ways - first, the on_message_send_complete callback must be idempotent (which the amqp_management on_message_send_complete is not) and secondly, the message has already been freed by the time the link attempts to settle it.

By moving the indicate_all_messages_as_error call to after the attempted detach, if the link fails to detach it will settle any pending messages and remove them from the message sender, thereby eliminating the double callback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant