From 59d11b2c3839864f57bd85ef5052d60ce17c5e1c Mon Sep 17 00:00:00 2001
From: Philip Withnall <philip.withnall@collabora.co.uk>
Date: Tue, 15 Dec 2015 11:20:27 +0000
Subject: [PATCH] tumbler: Fix potential race condition between Ready and Error
signals
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
As explained in the new comment in the Tumbler mixin, it is possible for
the Error and Ready signals from tumblerd to race if one thumbnailer
fails fast while another succeeds (for the same URI) slowly. This
happened for me with the cover and gst thumbnailers on
big_buck_bunny_smaller.ogv — the cover thumbnailer failed fast because
the search terms ‘big buck bunny smaller’ return nothing on omdbapi.com,
while the gst thumbnailer eventually succeeded by grabbing a frame from
the video.
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Differential Revision: https://phabricator.apertis.org/D1299
---
apertis_tests_lib/tumbler.py | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/apertis_tests_lib/tumbler.py b/apertis_tests_lib/tumbler.py
index 74862efe..17661ce9 100644
--- a/apertis_tests_lib/tumbler.py
+++ b/apertis_tests_lib/tumbler.py
@@ -30,6 +30,20 @@ class TumblerMixin:
None)
self.tumbler.connect('g-signal', self.__g_signal_cb)
+ # Track pending errors. Tumbler emits three signals which we care
+ # about:
+ # - Ready, which indicates successful thumbnailing for some URIs
+ # - Error, which indicates errors in thumbnailing for some URIs
+ # - Finished, which is the final signal emitted for a request
+ # Ready and Error may both be emitted for the same URI, for example if
+ # one of the thumbnailers encounters an error when thumbnailing that
+ # URI, but another thumbnailer subsequently succeeds.
+ # Therefore we must track errors and only propagate them when Finished
+ # is received.
+ #
+ # Each element of this list of a tuple: (uri, error_message).
+ self.tumbler_errors = []
+
# Spy on unicast signals that weren't meant for us, because
# the Thumbnailer API uses those, and we want to use them to
# determine when it has finished. GDBus doesn't have
@@ -54,13 +68,30 @@ class TumblerMixin:
elif signal_name == 'Finished':
child = parameters.get_child_value(0)
self.tumbler_queue.remove(child.get_uint32())
+
+ # Any errors remaining?
+ if len(self.tumbler_errors) > 0:
+ l = [': '.join(p) for p in self.tumbler_errors]
+ raise Exception("Error creating thumbnails: %s" % ', '.join(l))
+
+ # Clear them.
+ self.tumbler_errors = []
+ elif signal_name == 'Ready':
+ child = parameters.get_child_value(1)
+ uris = child.get_strv()
+
+ # Remove any now-successful URIs from the error list.
+ self.tumbler_errors = [p for p in self.tumbler_errors
+ if p[0] not in uris]
elif signal_name == 'Error':
child = parameters.get_child_value(1)
uris = child.get_strv()
child = parameters.get_child_value(3)
msg = child.get_string()
- raise Exception("Error creating thumbnail for %s: %s" %
- ', '.join(uris), msg)
+
+ # Append to the error list.
+ for uri in uris:
+ self.tumbler_errors.append((uri, msg))
def __get_supported_cb(self, source, result):
self.tumbler.call_finish(result)
--
GitLab