-
Notifications
You must be signed in to change notification settings - Fork 19
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 an issue where links are kept alive #115
Fix an issue where links are kept alive #115
Conversation
even though we are unable to create an Amqp receiver due to operation timeout. The problem is that `rheaReceiver` and `receiver` are created when the Promise instance is created, however, they are not removed when rejecting due to operation timeout. So the created objects are kept by `rhea` as long as the connection is alive. In this case there's no way for outside caller to do the clean up because `receiver` is not returned by the `resolve` callback. This PR adds cleanup for the `actionAfterTimeout` code path.
@@ -396,6 +396,15 @@ export class Session extends Entity { | |||
const msg: string = `Unable to create the amqp receiver '${receiver.name}' on amqp ` + | |||
`session '${this.id}' due to operation timeout.`; | |||
log.error("[%s] %s", this.connection.id, msg); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are reject
cases above too but I'd like to limit the scope of this PR to just timeout for now.
const createReceiverOptions = options as CreateReceiverOptions; | ||
if (createReceiverOptions?.session?.createReceiver) { | ||
// being called on a session passed via the options so don't close the session | ||
receiver.close({ closeSession: false }).then(() => { receiver.remove(); }) | ||
} else { | ||
receiver.close({ closeSession: true }).then(() => { receiver.remove(); }) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NIT]
Should this just be abstracted into a receiverCleanup
method so that it can be reused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! But could wait until it is going to be re-used.
even though we are unable to create an Amqp receiver due to operation timeout.
The problem is that
rheaReceiver
andreceiver
are created when the Promiseinstance is created, however, they are not removed when rejecting due to
operation timeout. So the created objects are kept by
rhea
as long as theconnection is alive but not usable by any. In this case there's no way for outside
caller to do the clean up because
receiver
is not returned by theresolve
callback.This PR adds cleanup for the
actionAfterTimeout
code path.