Skip to content

Conversation

@beatgammit
Copy link
Collaborator

  • allows us to generate session-specific ids

I basically rewrote dealer.go on a rereading of the spec. I was getting errors with AutobahnPython saying that the INVOCATION ID was reused (was causing somewhat random crashes). We were using NewID() to generate this, which generates the ID randomly, so I switched it to a session-specific, incrementing ID as per the spec. I also simplified some of the internal data structures.

Moving the *Session into the Dealer is a breaking change and was needed to generate session-specific identifiers for the fix. We talked about this previously, and we had decided not to do it because there wasn't a reason to complicate matters, but I think this is now necessary to be spec compliant.

If this looks good to you, I can check if our ids are correct elsewhere.

- allows us to generate session-specific ids
@beatgammit
Copy link
Collaborator Author

This is the fix for #85

- fixes a problem when running with AutobahnPython and twisted
- prevent data race; maps aren't safe for concurrent use
@mourad
Copy link
Collaborator

mourad commented May 4, 2016

IMO, it does makes sense to make this API change. I also found this change is needed in order to add smarter external router/broker (i.e. make routing decision based on session data)

The beatgammit:v2 branch has moved forward since this PR was created and now includes a lot of other changes. Could you change this PR to only include the original changes?

beatgammit and others added 14 commits August 1, 2016 13:36
- fixes problems with multiple subscribers to the same topic
- no guarantee they still work, but this makes it easier to run tests
- single rw lock to make eliminate races, could definitely use some
  performance tuning/refactoring
- it seems like this was blocking when the socket was already closed,
  which caused some stability issues over time as goroutines built up
- I noticed a that defaultBroker.Subscribe could block, and the only way
  that could happen is if the Send function in either Unsubscribe
  (unlikely) or Publish (very likely) blocked
- I think there's still a larger problem with transports blocking on the
  Send, but this should at least allow messages to go through if that
  happens
- fixes memory leak
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.

4 participants