GNOME Bugzilla – Bug 682527
Epiphany sometimes fails to exit immediately with "Cache flush finished despite X pending requests"
Last modified: 2013-02-18 10:26:30 UTC
Sometimes the SoupCache forces programs like Epy to wait for some seconds on exit with the message: "Cache flush finished despite X pending requests" That message means that the cache has some pending requests to handle. That happens to often and even if the network works properly so there must be some bug there. After some investigation I noticed that when the SoupSession client cancels some message (for example WebKitGtk does that when dealing with multimedia resources) then the SoupCache doesn't get notified about that and thus it will wait forever (because no write callback will be called).
I found a way to reliably got that bad behaviour loading this URL in epy: http://media.lavozdegalicia.es/default/2012/06/16/02101339850626718631188/Fichero/envejecimiento90h264.mp4
Created attachment 222231 [details] [review] Patch Check on message finalization for message cancellation. If that's the case then cancel the pending operations and close the stream.
Created attachment 224133 [details] [review] Patch for streamable SoupCache This version of the patch applies on top of the the proposed changes in bug 682112
Comment on attachment 224133 [details] [review] Patch for streamable SoupCache >+ /* Check if the message was cancelled from the outside. In >+ * that case we must finish all the caching here. >+ */ I think there's an easier way to do this; you just need to implement close() in SoupCacheInputStream, and do all the finishing/cleanup from there.
(In reply to comment #4) > (From update of attachment 224133 [details] [review]) > >+ /* Check if the message was cancelled from the outside. In > >+ * that case we must finish all the caching here. > >+ */ > > I think there's an easier way to do this; you just need to implement close() in > SoupCacheInputStream, and do all the finishing/cleanup from there. The thing is that SoupCacheInputStream has (and should have) zero knowledge about SoupMessages or SoupSessions. How would you check that the SoupMessage associated to the stream was cancelled then?
Does it actually need to know if the message was cancelled as opposed to successful? It looks like all you need to know is that it's "done", and the stream being closed will tell you that. (Although... shouldn't the cache file be discarded if the stream was cancelled?)
(In reply to comment #6) > Does it actually need to know if the message was cancelled as opposed to > successful? It looks like all you need to know is that it's "done", and the > stream being closed will tell you that. The problem with that is that the client could close the stream (because it had already read all the data) while there are still some data to be written to disk (because it's an async operation). Although... that's something that could happen currently as well... > (Although... shouldn't the cache file be discarded if the stream was > cancelled?) The problem described in this bug is not about a client cancelling the stream, but a client cancelling the SoupMessage. An example is how WebKitGtk+ handles multimedia streams. The first SoupMessage is issued by the WebKitGtk+ network layer but as soon as that message is received (and interpreted as a media element) then the original request is cancelled and some other code takes over the resource handling (and that cancellation is a soup_message_cancel as that code has no access to the streams used to read the resources).
Created attachment 236300 [details] [review] Fix for the bug
Created attachment 236301 [details] [review] Added a new flag for the cancellation tests
Created attachment 236302 [details] [review] New test for the bug
Comment on attachment 236300 [details] [review] Fix for the bug >+ signals[CACHING_FINISHED] = >+ g_signal_new ("caching-finished", >+ G_OBJECT_CLASS_TYPE (gobject_class), >+ G_SIGNAL_RUN_FIRST, >+ G_STRUCT_OFFSET (SoupCacheInputStreamClass, caching_finished), >+ NULL, NULL, >+ _soup_marshal_NONE__INT_POINTER, >+ G_TYPE_NONE, 2, >+ G_TYPE_INT, G_TYPE_ERROR); should be _soup_marshal_NONE__INT_BOXED... it's possible that that has no effect on the generated code, but anyway, it's more correct. otherwise looks good for now; at some point I've got to make caching work with the sync APIs too... I should file a bug about that
Closing then