GNOME Bugzilla – Bug 725209
lua-factory: Detect broken sources
Last modified: 2014-03-19 11:32:18 UTC
I was wondering why resolve_cb was never getting called for a specific media. The problem was a missing "grl.callback()" in another source that couldn't handle the resolution but bailed out without ever calling grl.callback(). We can actually detect broken sources in lua because every time we call into Lua either the resolve(), browse() or search() is finished (grl.callback() was called with a "0" remaining value), or an async fetch was started. We should finish the operation forcefully and print a warning when grl.callback() wasn't called and no fetch was started.
This is not a specific problem of Lua plugins, but a general problem: - A source doesn't invoke the callback - In browse/search operations, a source doesn't invoke the final callback to notify the end of the operation (that is, invoking with remaining == 0). The question is how to detect this, because depending on the source, some operations can take more time than others. My proposal is configure in a global manner a timeout option, so if we do not get the callback before the timeout expires, we assume the operation is already done. This option wouldn't be a plugin option, but a grilo option, as is the core who should control which sources are answering or not.
(In reply to comment #1) > This is not a specific problem of Lua plugins, but a general problem: That we cannot solve for C sources... In Lua, we can do this because all the I/O goes through the C layer where we can do our own accounting. We can't do this in the C sources because the libraries are called directly. If we need to modify the sources to take advantage of that, we might as well fix those sources...
But core invokes all the sources, so all callbacks passes through core. So we could account it there.
(In reply to comment #3) > But core invokes all the sources, so all callbacks passes through core. So we > could account it there. You wouldn't know whether there's anything pending. It might be waiting on a callback from GIO, libsoup, D-Bus, an ioctl, libcurl, or anything else really. So you would either have to proxy all the calls through another grilo function that did that, meaning that you would be fixing the source already, or you don't proxy those calls, and have no way of knowing what the source did, or if it's expecting something.
Well, the Lua plugin could be broken for any other reason that is not related with I/O. Could be it enters on an never-ending loop, or just the developer made a mistake so it never calls the callback with remaining==0. Core knows there is something pending: a operation in a source. But of course it doesn't know why it is pending, or if its going to end. The most it can do is wait for sometime and continue ahead without the problematic source/operation. I understand you're proposal is more specific for those cases where it the pending is due a specific I/O operation. I think the timeout option would not only helps to fix this problem, but also getting rid of those sources that are making things really slow. The application can still control the timeout value, so it can balance between getting faster answers (at the cost of leaving information out) and getting more complete answers (at the cost of taking more time).
Created attachment 272258 [details] [review] lua-factory: Protect against broken sources If no fetches are pending and grl.callback() was not called, throw a warning, and clean up after the source to avoid inconsistent states in front-ends.
Review of attachment 272258 [details] [review]: Nice! Found a bug in grl-metrolycs with it.
Attachment 272258 [details] pushed as 3ceffa5 - lua-factory: Protect against broken sources