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

m_subscriptions is not cleaned up when closing a session #39

Open
reddwarf69 opened this issue Jun 18, 2018 · 1 comment
Open

m_subscriptions is not cleaned up when closing a session #39

reddwarf69 opened this issue Jun 18, 2018 · 1 comment

Comments

@reddwarf69
Copy link

reddwarf69 commented Jun 18, 2018

I would have expected m_subscriptions to have been cleaned up by the time I get the closed_future.
Not sure if this could be a more serious issue (can the session be reopened and have old sessions? could it be considered a memory leak?). But for me it's an issue because my on_event_fn (indirectly) contains a reference to my wamp_session shared_ptr. So if I don't make sure to UNSUBSCRIBE all the subscriptions before closing the wamp_session the std::shared_ptr<wamp_session> reference counter never reaches zero and wamp_session is never destroyed.

Edit: Actually AFAICS the subscription in only removed from m_subscriptions in process_inbound_unsubscribed(). So I depend on my peer behaving and sending me the reply. If the connection dies for some reason and I can't get the UNSUBSCRIBED message back I'm out of luck.

@darrenjs
Copy link
Owner

Hi ... how about storing a weak_ptr inside your on_event_fn instead of the shared_ptr. The problem is the underlying connection can terminate uncleanly at any point (eg network disconnect), and I'm not sure it would make sense to invoke the on_event_fn to indicate an UNSUBSCRIBED has been received.

Or maybe, the library could be altered to release all internal state upon internal closure (e.g., I think this could be done in wampcc::transition_to_closed, although there might be synchronisation considerations to bare in mind, e.g., what locks need to be taken etc).

But, back to my initial thought. I think it's wrong to use a shared_pointer as you are doing. Shared pointers convey resource ownership semantics, and so by the wamp session holding a reference to itself, it kinds of means it controls its own lifetime! So I think weak pointer will work, you just need to promote it to shared pointer during each callback (infact, a raw pointer would work also).

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

No branches or pull requests

2 participants