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

Prevent duplicate subscription ID's in destinationCache of DefaultSubscriptionRegistry #32625

Closed
heowc opened this issue Apr 12, 2024 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@heowc
Copy link
Contributor

heowc commented Apr 12, 2024

Affects: 6.1.5


hello.

I encountered a problem with multiple subscriptions while delivering broadcast messages to subscribed sessions (clients), and I thought there might be a need for a way to prevent this, so I created an issue thinking it would be good to discuss.

My case is as follows:

  1. For some reason, subscription request was made twice for subscription A with the same ID (sub-1).
  2. The server stores it in memory as shown below.
    • sessionRegistry is {session-id, sub-1, subscription}
    • desctinationCache is {A, session-id, {sub-1,sub-1}}
  3. If you cancel subscription A
    • Find session information by session-id in sessionRegistry and remove subscription by sub-1
    • Get a list of all session-ids corresponding to desctination from desctinationCache and remove only one identical subscription ID from it.

In the above situation, sub-1, which cannot be erased, remains in desctinationCache. Of course, the client implementation may be incorrect, but it seems that this could lead to a situation in which excessive memory may accumulate on the server.

To think briefly, I think it would be a good idea to store the subscription list in desctinationCache as a collection that does not allow duplication, such as a hashset, rather than an array list. Or is there a reason it has to be this way or is there another better way?

I see a few similar issues, but they seem slightly different.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 12, 2024
@snicoll snicoll added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Apr 16, 2024
@heowc
Copy link
Contributor Author

heowc commented Apr 17, 2024

It varies depending on the implementation, but vertx-stomp limits this.

@rstoyanchev
Copy link
Contributor

SessionInfo.addSubscription uses computeIfAbsent, essentially ignoring an already existing registration. It's only the destination cache that accumulates multiple subscription id's in the same session. That's not intentional.

@rstoyanchev rstoyanchev self-assigned this Apr 18, 2024
@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 18, 2024
@rstoyanchev rstoyanchev added this to the 6.1.7 milestone Apr 18, 2024
@rstoyanchev rstoyanchev changed the title Prevent subscription with same subscription ID in websocket Prevent duplicate subscription ID's in destinationCache of DefaultSubscriptionRegistry Apr 18, 2024
@jhoeller jhoeller added the type: enhancement A general enhancement label Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants