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

[service-bus] Logic for removing links from our internal context map is incorrect and does not remove links #15890

Closed
richardpark-msft opened this issue Jun 22, 2021 · 0 comments · Fixed by #15929
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Service Bus

Comments

@richardpark-msft
Copy link
Member

Found through inspection looking at another issue.

This code uses the wrong values for this._linkType:

    this._logger.verbose(`${this.logPrefix} permanently closing this link.`);

    // Remove the underlying AMQP link from the cache
    switch (this._linkType) {
      case "s": {
        delete this._context.senders[this.name];
        break;
      }
      case "br":
      case "sr": {
        delete this._context.messageReceivers[this.name];
        break;
      }
      case "ms": {
        delete this._context.messageSessions[this.name];
        break;
      }
    }

However, the values for _linkType are not 's', 'br', 'sr', and 'ms' - they're actually the longer form of each of those names: "batching" | "streaming" | "session" | "mgmt":

type LinkTypeT<
  LinkT extends Receiver | AwaitableSender | RequestResponseLink
> = LinkT extends Receiver
  ? ReceiverType
  : LinkT extends AwaitableSender
  ? "sender" // sender
  : LinkT extends RequestResponseLink
  ? "mgmt" // management link
  : never;

export type NonSessionReceiverType =
  | "batching" // batching receiver
  | "streaming"; // streaming receiver

export type ReceiverType = NonSessionReceiverType | "session"; // message session

This might not be a direct culprit in some bugs (the links, I believe, do get closed, it's just not cleaned up from our dictionary) but it definitely isn't correct and should be fixed.

@richardpark-msft richardpark-msft added Client This issue points to a problem in the data-plane of the library. Service Bus labels Jun 22, 2021
richardpark-msft added a commit that referenced this issue Jun 24, 2021
…rnal cache (#15929)

Today we cache any opened links in the connectionContext. These links should be removed when the link itself is closed but, due to a mismatch in the values, we weren't. 

I've fixed this by just making an abstract method in LinkEntity (the base for all the link types) and just having each link properly remove itself from the cache.

Fixes #15890
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. Service Bus
Projects
None yet
2 participants