| Summary: | Close() on MUC channels should probably be asynchronous | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Will Thompson <will> |
| Component: | gabble | Assignee: | Telepathy bugs list <telepathy-bugs> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | CC: | sjoerd |
| Version: | unspecified | Keywords: | patch |
| Hardware: | Other | ||
| OS: | All | ||
| URL: | http://git.collabora.co.uk/?p=user/jonny/telepathy-gabble.git;a=shortlog;h=refs/heads/close-mucs-async | ||
| Whiteboard: | review+ with one change | ||
| i915 platform: | i915 features: | ||
|
Description
Will Thompson
2009-02-03 06:23:15 UTC
Okay I wrote a branch for this, and even a test! One problem might be that there is currently no timeout on this. If a server doesn't echo your unavailable presence then the channel will never close. I'm not sure if this is a problem yet though. Thoughts? I think your test should check that Gabble does *something* sensible if you request a new channel while it's waiting for the server to ack you leaving the room.
Possible sensible things include:
1. Just failing the channel request.
2. Queuing the join request.
> One problem might be that there is currently no timeout on this. If a server
> doesn't echo your unavailable presence then the channel will never close. I'm
> not sure if this is a problem yet though. Thoughts?
I think that, given that Gabble bothers to have a 180-second (!) timeout for joining a room in the first place, it should have the same for leaving it.
Oh, and does this deal correctly with: 1. UI calls Close() 2. UI calls Disconnect() 3. Server echoes our presence to say we've left the room Definitions of correctly include: • The channel emits Closed() when Disconnect() is called; and doesn't crash when the presence arrives. I believe this works by accident: when Disconnect() is called, the muc factory tells the channel to close without waiting, and then unrefs the channel, which happens to destroy it, which has the effect of destroying the WockyMuc so WockyMuc::parted will not fire and crash us. • The channel doesn't emit closed until either the presence is echoed, or Gabble gets bored of waiting for the stream close from the server. (In reply to comment #2) > I think your test should check that Gabble does *something* sensible if you > request a new channel while it's waiting for the server to ack you leaving the > room. Makes sense. I've changed the test to ensure it fails with NotAvailable, and also that it works after the channel has finally closed. > I think that, given that Gabble bothers to have a 180-second (!) timeout for > joining a room in the first place, it should have the same for leaving it. Okay, added. I made it 180s again for consistency. I'm not sure I'd be so popular if I made a test for that. (In reply to comment #3) > Oh, and does this deal correctly with: [snip] > • The channel emits Closed() when Disconnect() is called; and doesn't crash > when the presence arrives. I believe this works by accident: when Disconnect() > is called, the muc factory tells the channel to close without waiting, and then > unrefs the channel, which happens to destroy it, which has the effect of > destroying the WockyMuc so WockyMuc::parted will not fire and crash us. Yeah that's what happened. I added a test for this case as you saw IRL. const char *jid = wocky_muc_jid (wmuc);
+ DEBUG ("called");
+
Come on. The JID is right there.
otherwise this looks fine le singe est sur la table |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.