GNOME Bugzilla – Bug 726904
Previewers should not use GLib.source_remove(), use boolean functions instead
Last modified: 2014-09-27 20:33:04 UTC
While reading the code, noticed that at pitivi/timeline/previewers.py the timeout is hard-removed using GLib.remove_source(). On my system, this results in the following lines: /opt/gnome/lib64/pitivi/python/pitivi/timeline/previewers.py:801: Warning: Source ID 2554 was not found when attempting to remove it GLib.source_remove(self._callback_id) /opt/gnome/lib64/pitivi/python/pitivi/timeline/previewers.py:801: Warning: Source ID 2577 was not found when attempting to remove it GLib.source_remove(self._callback_id) /opt/gnome/lib64/pitivi/python/pitivi/timeline/previewers.py:801: Warning: Source ID 2581 was not found when attempting to remove it GLib.source_remove(self._callback_id) /opt/gnome/lib64/pitivi/python/pitivi/timeline/previewers.py:801: Warning: Source ID 2589 was not found when attempting to remove it GLib.source_remove(self._callback_id) This is easily avoidable simply by using boolean functions. GLib.timeout_add() automatically stops running when it returns False, and keep running while it returns True. Using boolean functions we aviod having to deal with removing resources manually (which are not thread safe, AFAIK).
For me GLib.timeout_add() return an ID of the source, as documented here: https://developer.gnome.org/pygobject/stable/glib-functions.html#function-glib--timeout-add (Sorry didn't find the current documentation, but the function seems to be a GI override, since it does not appear to be in GLib.) My shell experiment supports this: In [1]: from gi.repository import GLib In [2]: def foo(): ...: print("lol") ...: In [3]: GLib.timeout_add(1000, foo) Out[3]: 1 In [4]: GLib.timeout_add(1000, foo) Out[4]: 2 In [5]: GLib.timeout_add(1000, foo) Out[5]: 3 In [6]: GLib.source_remove(1) Out[6]: True In [7]: GLib.source_remove(2) Out[7]: True In [8]: GLib.source_remove(3) Out[8]: True =================================== In Pitivi, there is a check wheather the source is false, but it is also true if its an invalid ID. if self._callback_id: GLib.source_remove(self._callback_id) self._callback_id = GLib.timeout_add(500, self._compute_geometry) The problem is that IDs are invalid, since it seems to happen ansyc. Do you suggest just removing the "GLib.source_remove" call? Since you state that the process stops automatically.
Running the _compute_geometry method synchronously fixes a UI Bug for me, so maybe it is not needed at all: https://github.com/lubosz/pitivi/commits/previewer
Those warnings are suspicious and I'd rather not have them so we can eliminate that being a possibile cause of bugs.
Lubosz, your three commits seem to work in practice, and I have not seen the UI block - isn't having this synchronous potentially problematic though? If you're certain that this does not need to be async, we could squash this into one commit and merge it.
I suppose this is the only commit that got merged related to this? commit ae0ef1fa9052c9d071da7df25c68c0ada2d0d17f Author: Lubosz Sarnecki <lubosz@gmail.com> Date: Fri Apr 11 22:14:41 2014 +0200 video previewer: remove unused self._callback_id