Skip to content
This repository has been archived by the owner on Jan 9, 2018. It is now read-only.

Should single Dispatcher be shared across all endpoints? #34

Open
trumpetinc opened this issue Feb 20, 2014 · 0 comments
Open

Should single Dispatcher be shared across all endpoints? #34

trumpetinc opened this issue Feb 20, 2014 · 0 comments

Comments

@trumpetinc
Copy link
Collaborator

I'm seeing the following scenario as a strong possibility:

  1. Single dispatcher gets created in WampServerEndpointConfig
  2. Client connects, server constructs a new WampServerEndpoint (with the dispatcher)
  3. this calls dispatcher.newConnection(session), which in turn iterates all of the handlers and calls WampMessageHandler.onOpen with the current session id

DefaultRPCSender and DefaultEventSubscriber have onOpen implementations that store the session id, like this (this is from DefaultRPCSender):

@Override
public void onOpen(String sessionId) {
    this.sessionId = sessionId;
}

every time a new client connects, the session id stored in each of these handlers is going to change.

I'm pretty sure that bad things will happen as a result.

The fix seems to be to not share Dispatcher across all endpoints. Instead, each session should get it's own dispatcher, so we should configure the ClientEndpoint and ServerEndpointConfig objects with a DispatcherFactory instead. When WampSessions are created, we also create a dispatcher.

What's nice about this change is that dispatcher can be tightly bound to the WampSession object (in fact, it can be a member of WampSession). I believe that this would simplify quite a few of the code paths through jWAMP. For example, instead of passing sessionId around, we can pass the session itself:

instead of

Dispatcher.onMessage(WampMessage, String)

we would have:

Dispatcher.onMessage(WampMessage, WampSession)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant