|Summary:||Porter: _close_async() should take a timeout, and _force_close_async() should not be API|
|Product:||Wocky||Reporter:||Will Thompson <will>|
|Component:||General||Assignee:||Telepathy bugs list <telepathy-bugs>|
|Status:||NEW ---||QA Contact:||Telepathy bugs list <telepathy-bugs>|
|i915 platform:||i915 features:|
|Bug Depends on:|
Description Will Thompson 2011-03-22 08:48:52 UTC
It's silly that applications which call _close_async() cannot rely on it ever completing, and so have to mess around adding their own arbitrary timeout and then calling _force_close_async() and then when their callback for _close_async() is called they have to check whether the reason it failed was because it was forcibly closed (by them!) and if so treat the error as not an error. I think _close_async() should take a timeout, with a sensible default and a way to say “RIGHT NOW DON'T WAIT”. _close_finish() should return a flag for whether the connection was closed cleanly. In fact, the only reason _close_async() should fail is if the porter is already closed when you call it, as I see it. (And arguably then it should just succeed.) This would dramatically simplify the code in Gabble and Salut which closes up porters, at the cost of slightly complicating the porter implementation, but I think it would be easier to implement within the porter than without.
Comment 1 Jonny Lamb 2011-03-23 03:30:00 UTC
I highly endorse this initiative and am about to start working on a solution.
Comment 2 Will Thompson 2013-02-15 17:56:56 UTC
! I just discovered that when the server sends you </stream:stream> unexpectedly, the application is still expected to call wocky_porter_close_async(). Seems pretty silly. As far as I can tell, apps need to listen for remote-closed and remote-error, and when those signals are emitted, call wocky_porter_close_async(), but only if they haven't already locally closed the connection (well, they could call it in that situation too but then it will fail). I suppose the client does still need to have some way to know when all its queued stanzas have been sent, and having to call _close_async() does provide that. Still feels awkward.
Comment 3 Will Thompson 2013-02-18 16:14:02 UTC
It might be cleanest to make _close_async() pass its GCancellable down to the XmppConnection and do like elsewhere in GIO does: if you cancel close-ing a thing, or pass an already cancelled cancellable in, you get a forced, unclean close.