GNOME Bugzilla – Bug 673468
SoupClientContext is accessed after being disposed
Last modified: 2012-04-04 17:02:56 UTC
Created attachment 211252 [details] backtrace We use libsoup in telepathy-salut to do OOB file transfers using the amusing iChat HTTP-based protocol. Sometimes in a file transfer test Salut will crash with the attached backtrace. It only happens once every so often so is very hard to reproduce. Looking at the trace it seems to go something like this though: 1. Salut (Gibber) unrefs the SoupServer 2. the server goes through all its clients (SoupClientContext structs) and sets the client's message as finished 3. the message being finished means the callback to soup_message_read_request is called, 'request_finished' 4. request_finished unrefs the last ref on the client context, and so frees the struct 5. back into the dispose function loop, the client's socket gets disconnected 6. SoupSocket does some clearing up upon being disconnected and emits ::disconnected 5. SoupServer, still connected to SoupSocket::disconnected, has its 'socket_disconnected' callback called 6. this callback uses the already freed SoupClientContext struct and so kaboom. It seems the easiest way to fix this is to hold a ref on the SoupClientContext struct for the duration of the cleanup so we can set the message as finished *and* disconnect the socket without disposing the client. Patch on its way.
Created attachment 211253 [details] [review] server: keep a ref on the client context while clearing up What do you think? Am I missing something here? I'm running the Salut test that would sometimes fail over and over again and no failures so far.
Comment on attachment 211253 [details] [review] server: keep a ref on the client context while clearing up Makes sense (assuming that libsoup's own "make check" still passes). One super minor nitpick: >+ /* keep a ref on the client context so it doesn't get destroyed >+ * when we finish the message; the SoupSocket::disconnect >+ * handler will refer to client->server later when the socket is >+ * disconnected. */ >+ soup_client_context_ref (client); The closing "*/" should be on the next line
(In reply to comment #2) > (From update of attachment 211253 [details] [review]) > Makes sense (assuming that libsoup's own "make check" still passes). Yep. > The closing "*/" should be on the next line Fixed and pushed, thanks!