Merge lp:~jamesh/thumbnailer/dbus-handler into lp:thumbnailer/devel

[James Henstridge]

Split out the D-Bus method request handlers into separate classes, with some processing happening in a thread pool (where blocking is permissible) and some in the event loop thread (where blocking is not allowed, but we don't monopolise a thread).

Split the D-Bus method handlers out from the DBusInterface class, and structure them so they can run in three phases. The intended phases are:

1. check() - called in a thread pool, so can perform blocking calls. This method should determine whether it has a valid image available in the cache, and return it if so.

May raise exceptions on failure. If no image is returned, the request moves on to phase 2.

2. download() - called from the event loop, so can not block. This phase is intended to be used to perform actions that don't require a dedicated thread to complete, such as using QNetwork to download data, or QProcess to monitor a child process.

When complete, the handler should call the downloadFinished() slot on success, or sendError() method on failure.

3. check() - called in a thread pool, so can perform blocking calls. This method should use the information gathered from phase 2 to generate a thumbnail, store it in the cache, and return it to the user.

As with phase 1, exceptions can be used to signal failure.

At the moment, none of the code from the Thumbnailer class is set up to handle this split of responsibility, so the first two phases are stubbed out with all the work being done in the third phase. I could have done the work in the first phase, but this ensures that the other two are getting called correctly.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:135
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~jamesh/thumbnailer/dbus-handler/+merge/256616/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/31/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-amd64-ci/31
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/31
        deb: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/31/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-i386-ci/31

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-devel-ci/31/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:136
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~jamesh/thumbnailer/dbus-handler/+merge/256616/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/33/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-amd64-ci/33/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/33/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-i386-ci/33/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-devel-ci/33/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:138
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~jamesh/thumbnailer/dbus-handler/+merge/256616/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/34/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-amd64-ci/34
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/34
        deb: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/34/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-i386-ci/34

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-devel-ci/34/rebuild

review: Needs Fixing (continuous-integration)

[Michi Henning (michihenning)]

Looks very good, thanks!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-autolanding/14/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-amd64-autolanding/14/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-autolanding/14/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-i386-autolanding/15/console

review: Needs Fixing (continuous-integration)

[lp:~jamesh/thumbnailer/dbus-handler]

139. By James Henstridge on 2015-04-20

Merge from devel, fixing conflicts.

[Michi Henning (michihenning)]

Thanks!

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:139
http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/36/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-amd64-ci/36
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/36
        deb: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/36/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-i386-ci/36

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-devel-ci/36/rebuild

review: Approve (continuous-integration)

=== modified file 'src/service/CMakeLists.txt'
--- src/service/CMakeLists.txt	2015-04-13 04:15:17 +0000
+++ src/service/CMakeLists.txt	2015-04-20 05:02:20 +0000
@@ -1,15 +1,19 @@
 add_definitions(${THUMBNAILER_CFLAGS})
 include_directories(. ${CMAKE_CURRENT_BINARY_DIR})
 
-qt5_add_dbus_adaptor(adaptor_files dbusinterface.xml dbusinterface.h DBusInterface)
+qt5_add_dbus_adaptor(adaptor_files dbusinterface.xml dbusinterface.h unity::thumbnailer::service::DBusInterface)
 
 add_executable(thumbnailer-service
   main.cpp
   dbusinterface.cpp
+  handler.cpp
+  thumbnailhandler.cpp
+  albumarthandler.cpp
+  artistarthandler.cpp
   ${adaptor_files}
   )
 
-qt5_use_modules(thumbnailer-service DBus)
+qt5_use_modules(thumbnailer-service DBus Concurrent)
 target_link_libraries(thumbnailer-service thumbnailer ${CMAKE_THREAD_LIBS_INIT})
 set_target_properties(thumbnailer-service PROPERTIES AUTOMOC TRUE)
 

=== added file 'src/service/albumarthandler.cpp'
--- src/service/albumarthandler.cpp	1970-01-01 00:00:00 +0000
+++ src/service/albumarthandler.cpp	2015-04-20 05:02:20 +0000
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * Authors:
+ *    James Henstridge <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or modify it under
+ * the terms of version 3 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "albumarthandler.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <unity/util/ResourcePtr.h>
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct AlbumArtHandlerPrivate {
+    const std::shared_ptr<Thumbnailer> thumbnailer;
+    const QString artist;
+    const QString album;
+    const ThumbnailSize size;
+
+    AlbumArtHandlerPrivate(const std::shared_ptr<Thumbnailer> &thumbnailer,
+                           const QString &artist,
+                           const QString &album,
+                           ThumbnailSize size)
+        : thumbnailer(thumbnailer), artist(artist), album(album), size(size) {
+    }
+};
+
+AlbumArtHandler::AlbumArtHandler(const QDBusConnection &bus,
+                                 const QDBusMessage &message,
+                                 const std::shared_ptr<Thumbnailer> &thumbnailer,
+                                 const QString &artist,
+                                 const QString &album,
+                                 ThumbnailSize size)
+    : Handler(bus, message), p(new AlbumArtHandlerPrivate(thumbnailer, artist, album, size)) {
+}
+
+AlbumArtHandler::~AlbumArtHandler() {
+}
+
+QDBusUnixFileDescriptor AlbumArtHandler::check() {
+    return QDBusUnixFileDescriptor();
+}
+
+void AlbumArtHandler::download() {
+    downloadFinished();
+}
+
+QDBusUnixFileDescriptor AlbumArtHandler::create() {
+    std::string art_image = p->thumbnailer->get_album_art(
+        p->artist.toStdString(), p->album.toStdString(), p->size, TN_REMOTE);
+
+    if (art_image.empty()) {
+        throw std::runtime_error("Could not get thumbnail");
+    }
+
+    return write_to_tmpfile(art_image);
+}
+
+}
+}
+}

=== added file 'src/service/albumarthandler.h'
--- src/service/albumarthandler.h	1970-01-01 00:00:00 +0000
+++ src/service/albumarthandler.h	2015-04-20 05:02:20 +0000
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * Authors:
+ *    James Henstridge <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or modify it under
+ * the terms of version 3 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "handler.h"
+#include <thumbnailer.h>
+
+#include <memory>
+
+#include <QObject>
+#include <QDBusConnection>
+#include <QDBusMessage>
+#include <QDBusUnixFileDescriptor>
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct AlbumArtHandlerPrivate;
+
+class AlbumArtHandler : public Handler {
+    Q_OBJECT
+public:
+    AlbumArtHandler(const QDBusConnection &bus, const QDBusMessage &message,
+                    const std::shared_ptr<Thumbnailer> &thumbnailer,
+                    const QString &artist,
+                    const QString &album,
+                    ThumbnailSize size);
+    ~AlbumArtHandler();
+
+protected:
+    virtual QDBusUnixFileDescriptor check() override;
+    virtual void download() override;
+    virtual QDBusUnixFileDescriptor create() override;
+
+private:
+    std::unique_ptr<AlbumArtHandlerPrivate> p;
+};
+
+}
+}
+}

=== added file 'src/service/artistarthandler.cpp'
--- src/service/artistarthandler.cpp	1970-01-01 00:00:00 +0000
+++ src/service/artistarthandler.cpp	2015-04-20 05:02:20 +0000
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * Authors:
+ *    James Henstridge <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or modify it under
+ * the terms of version 3 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "artistarthandler.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <unity/util/ResourcePtr.h>
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct ArtistArtHandlerPrivate {
+    const std::shared_ptr<Thumbnailer> thumbnailer;
+    const QString artist;
+    const QString album;
+    const ThumbnailSize size;
+
+    ArtistArtHandlerPrivate(const std::shared_ptr<Thumbnailer> &thumbnailer,
+                            const QString &artist,
+                            const QString &album,
+                            ThumbnailSize size)
+        : thumbnailer(thumbnailer), artist(artist), album(album), size(size) {
+    }
+};
+
+ArtistArtHandler::ArtistArtHandler(const QDBusConnection &bus,
+                                   const QDBusMessage &message,
+                                   const std::shared_ptr<Thumbnailer> &thumbnailer,
+                                   const QString &artist,
+                                   const QString &album,
+                                   ThumbnailSize size)
+    : Handler(bus, message), p(new ArtistArtHandlerPrivate(thumbnailer, artist, album, size)) {
+}
+
+ArtistArtHandler::~ArtistArtHandler() {
+}
+
+QDBusUnixFileDescriptor ArtistArtHandler::check() {
+    return QDBusUnixFileDescriptor();
+}
+
+void ArtistArtHandler::download() {
+    downloadFinished();
+}
+
+QDBusUnixFileDescriptor ArtistArtHandler::create() {
+    std::string art_image = p->thumbnailer->get_artist_art(
+        p->artist.toStdString(), p->album.toStdString(), p->size, TN_REMOTE);
+
+    if (art_image.empty()) {
+        throw std::runtime_error("Could not get thumbnail");
+    }
+
+    return write_to_tmpfile(art_image);
+}
+
+}
+}
+}

=== added file 'src/service/artistarthandler.h'
--- src/service/artistarthandler.h	1970-01-01 00:00:00 +0000
+++ src/service/artistarthandler.h	2015-04-20 05:02:20 +0000
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * Authors:
+ *    James Henstridge <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or modify it under
+ * the terms of version 3 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "handler.h"
+#include <thumbnailer.h>
+
+#include <memory>
+
+#include <QObject>
+#include <QDBusConnection>
+#include <QDBusMessage>
+#include <QDBusUnixFileDescriptor>
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct ArtistArtHandlerPrivate;
+
+class ArtistArtHandler : public Handler {
+    Q_OBJECT
+public:
+    ArtistArtHandler(const QDBusConnection &bus, const QDBusMessage &message,
+                    const std::shared_ptr<Thumbnailer> &thumbnailer,
+                    const QString &artist,
+                    const QString &album,
+                    ThumbnailSize size);
+    ~ArtistArtHandler();
+
+protected:
+    virtual QDBusUnixFileDescriptor check() override;
+    virtual void download() override;
+    virtual QDBusUnixFileDescriptor create() override;
+
+private:
+    std::unique_ptr<ArtistArtHandlerPrivate> p;
+};
+
+}
+}
+}

=== modified file 'src/service/dbusinterface.cpp'
--- src/service/dbusinterface.cpp	2015-04-16 12:39:55 +0000
+++ src/service/dbusinterface.cpp	2015-04-20 05:02:20 +0000
@@ -18,34 +18,23 @@
  */
 
 #include "dbusinterface.h"
-
+#include "thumbnailhandler.h"
+#include "albumarthandler.h"
+#include "artistarthandler.h"
 #include <thumbnailer.h>
 
-#include <internal/safe_strerror.h>
+#include <map>
 
 #include <QDBusConnection>
 #include <QDBusMessage>
 #include <QDebug>
-#include <QRunnable>
-#include <QThreadPool>
-#include <unity/util/ResourcePtr.h>
-
-#include <fcntl.h>
-#include <sys/stat.h>
-#include <unistd.h>
 
 using namespace std;
-using namespace unity::thumbnailer::internal;
-
-static const char ART_ERROR[] = "com.canonical.MediaScanner2.Error.Failed";
-
-struct DBusInterfacePrivate {
-    std::shared_ptr<Thumbnailer> thumbnailer = std::make_shared<Thumbnailer>();
-    QThreadPool pool;
-};
 
 namespace {
 
+const char ART_ERROR[] = "com.canonical.Thubnailer.Error.Failed";
+
 ThumbnailSize desiredSizeFromString(const QString &size)
 {
     if (size == "small") {
@@ -62,16 +51,16 @@
     throw std::logic_error(error);
 }
 
-class Task final : public QRunnable {
-public:
-    Task(std::function<void()> task) : task(task) {}
-    virtual void run() override {
-        task();
-    }
-private:
-    std::function<void()> task;
+}
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct DBusInterfacePrivate {
+    std::shared_ptr<Thumbnailer> thumbnailer = std::make_shared<Thumbnailer>();
+    std::map<Handler*,std::unique_ptr<Handler>> requests;
 };
-}
 
 DBusInterface::DBusInterface(QObject *parent)
     : QObject(parent), p(new DBusInterfacePrivate) {
@@ -80,48 +69,6 @@
 DBusInterface::~DBusInterface() {
 }
 
-namespace
-{
-
-QDBusUnixFileDescriptor write_to_tmpfile(string const& image)
-{
-
-    typedef unity::util::ResourcePtr<int, decltype(&::close)> FdPtr;
-
-    static auto find_tmpdir = []
-    {
-        char const* dirp = getenv("TMPDIR");
-        string dir = dirp ? dirp : "/tmp";
-        return dir;
-    };
-    static string dir = find_tmpdir();
-
-    int fd = open(dir.c_str(), O_TMPFILE | O_RDWR);
-    if (fd < 0) {
-        string err = "cannot create tmpfile in " + dir + ": " + safe_strerror(errno);
-        throw runtime_error(err);
-    }
-    FdPtr fd_ptr(fd, ::close);
-    auto rc = write(fd_ptr.get(), &image[0], image.size());
-    if (rc == -1)
-    {
-        string err = "cannot write image data in " + dir + ": " + safe_strerror(errno);
-        throw runtime_error(err);
-    }
-    else if (string::size_type(rc) != image.size())
-    {
-        string err = "short write for image data in " + dir +
-                     "(requested = " + to_string(image.size()) + ", actual = " + to_string(rc) + ")";
-        throw runtime_error(err);
-    }
-    lseek(fd, SEEK_SET, 0);  // No error check needed, can't fail.
-
-    return QDBusUnixFileDescriptor(fd_ptr.get());
-}
-
-}
-
-
 QDBusUnixFileDescriptor DBusInterface::GetAlbumArt(const QString &artist, const QString &album, const QString &desiredSize) {
     qDebug() << "Look up cover art for" << artist << "/" << album << "at size" << desiredSize;
 
@@ -133,37 +80,8 @@
         return QDBusUnixFileDescriptor();
     }
 
-    setDelayedReply(true);
-    auto thumbnailer = p->thumbnailer;
-    auto bus = connection();
-    auto msg = message();
-    p->pool.start(new Task([thumbnailer, artist, album, size, bus, msg]() {
-        std::string art_image;
-        try {
-            art_image = thumbnailer->get_album_art(
-                artist.toStdString(), album.toStdString(), size, TN_REMOTE);
-        } catch (const std::exception &e) {
-            bus.send(msg.createErrorReply(ART_ERROR, e.what()));
-            return;
-        }
-
-        if (art_image.empty()) {
-            bus.send(msg.createErrorReply(ART_ERROR, "Could not get thumbnail"));
-            return;
-        }
-
-        try
-        {
-            auto unix_fd = write_to_tmpfile(art_image);
-            bus.send(msg.createReply(QVariant::fromValue(unix_fd)));
-        }
-        catch (runtime_error const& e)
-        {
-            string err = string("GetAlbumArt(): ") + e.what();
-            bus.send(msg.createErrorReply(ART_ERROR, err.c_str()));
-        }
-    }));
-
+    queueRequest(new AlbumArtHandler(connection(), message(), p->thumbnailer,
+                                     artist, album, size));
     return QDBusUnixFileDescriptor();
 }
 
@@ -178,42 +96,13 @@
         return QDBusUnixFileDescriptor();
     }
 
-    setDelayedReply(true);
-    auto thumbnailer = p->thumbnailer;
-    auto bus = connection();
-    auto msg = message();
-    p->pool.start(new Task([thumbnailer, artist, album, size, bus, msg]() {
-        std::string art_image;
-        try {
-            art_image = thumbnailer->get_artist_art(
-                artist.toStdString(), album.toStdString(), size, TN_REMOTE);
-        } catch (const std::exception &e) {
-            bus.send(msg.createErrorReply(ART_ERROR, e.what()));
-            return;
-        }
-
-        if (art_image.empty()) {
-            bus.send(msg.createErrorReply(ART_ERROR, "Could not get thumbnail"));
-            return;
-        }
-
-        try
-        {
-            auto unix_fd = write_to_tmpfile(art_image);
-            bus.send(msg.createReply(QVariant::fromValue(unix_fd)));
-        }
-        catch (runtime_error const& e)
-        {
-            string err = string("GetArtistArt(): ") + e.what();
-            bus.send(msg.createErrorReply(ART_ERROR, err.c_str()));
-        }
-    }));
-
+    queueRequest(new ArtistArtHandler(connection(), message(), p->thumbnailer,
+                                      artist, album, size));
     return QDBusUnixFileDescriptor();
 }
 
 QDBusUnixFileDescriptor DBusInterface::GetThumbnail(const QString &filename, const QDBusUnixFileDescriptor &filename_fd, const QString &desiredSize) {
-    qDebug() << "Look thumbnail for" << filename << "at size" << desiredSize;
+    qDebug() << "Create thumbnail for" << filename << "at size" << desiredSize;
 
     ThumbnailSize size;
     try {
@@ -223,52 +112,31 @@
         return QDBusUnixFileDescriptor();
     }
 
-    struct stat filename_stat, fd_stat;
-    if (stat(filename.toUtf8(), &filename_stat) < 0) {
-        sendErrorReply(ART_ERROR, "Could not stat file");
-        return QDBusUnixFileDescriptor();
-    }
-    if (fstat(filename_fd.fileDescriptor(), &fd_stat) < 0) {
-        sendErrorReply(ART_ERROR, "Could not stat file descriptor");
-        return QDBusUnixFileDescriptor();
-    }
-    if (filename_stat.st_dev != fd_stat.st_dev ||
-        filename_stat.st_ino != fd_stat.st_ino) {
-        sendErrorReply(ART_ERROR, "filename refers to a different file to the file descriptor");
-        return QDBusUnixFileDescriptor();
-    }
+    queueRequest(new ThumbnailHandler(connection(), message(), p->thumbnailer,
+                                      filename, filename_fd, size));
+    return QDBusUnixFileDescriptor();
+}
 
+void DBusInterface::queueRequest(Handler *handler) {
+    p->requests.emplace(handler, std::unique_ptr<Handler>(handler));
+    connect(handler, &Handler::finished, this, &DBusInterface::requestFinished);
     setDelayedReply(true);
-    auto thumbnailer = p->thumbnailer;
-    auto bus = connection();
-    auto msg = message();
-    p->pool.start(new Task([=]() {
-        std::string art;
-        try {
-            art = thumbnailer->get_thumbnail(
-                filename.toStdString(), size, TN_REMOTE);
-        } catch (const std::exception &e) {
-            bus.send(msg.createErrorReply(ART_ERROR, e.what()));
-            return;
-        }
-
-        if (art.empty()) {
-            bus.send(msg.createErrorReply(ART_ERROR, "Could not get thumbnail"));
-            return;
-        }
-
-        // FIXME: check that the thumbnail was produced for fd_stat
-        int fd = open(art.c_str(), O_RDONLY);
-        if (fd < 0) {
-            bus.send(msg.createErrorReply(ART_ERROR, safe_strerror(errno).c_str()));
-            return;
-        }
-
-        QDBusUnixFileDescriptor unix_fd(fd);
-        close(fd);
-
-        bus.send(msg.createReply(QVariant::fromValue(unix_fd)));
-    }));
-
-    return QDBusUnixFileDescriptor();
+    handler->begin();
+}
+
+void DBusInterface::requestFinished() {
+    Handler *handler = static_cast<Handler*>(sender());
+    try {
+        auto &h = p->requests.at(handler);
+        h.release();
+        p->requests.erase(handler);
+    } catch (const std::out_of_range &e) {
+        qWarning() << "finished() called on unknown handler" << handler;
+    }
+    // Queue deletion of handler when we re-enter the event loop.
+    handler->deleteLater();
+}
+
+}
+}
 }

=== modified file 'src/service/dbusinterface.h'
--- src/service/dbusinterface.h	2015-04-13 01:58:39 +0000
+++ src/service/dbusinterface.h	2015-04-20 05:02:20 +0000
@@ -17,8 +17,9 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef DBUSINTERFACE_H
-#define DBUSINTERFACE_H
+#pragma once
+
+#include "handler.h"
 
 #include <memory>
 
@@ -27,6 +28,10 @@
 #include <QObject>
 #include <QString>
 
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
 struct DBusInterfacePrivate;
 
 class DBusInterface : public QObject, protected QDBusContext {
@@ -44,7 +49,15 @@
     QDBusUnixFileDescriptor GetThumbnail(const QString &filename, const QDBusUnixFileDescriptor &filename_fd, const QString &desiredSize);
 
 private:
+    void queueRequest(Handler* handler);
+
+private Q_SLOTS:
+    void requestFinished();
+
+private:
     std::unique_ptr<DBusInterfacePrivate> p;
 };
 
-#endif
+}
+}
+}

=== added file 'src/service/handler.cpp'
--- src/service/handler.cpp	1970-01-01 00:00:00 +0000
+++ src/service/handler.cpp	2015-04-20 05:02:20 +0000
@@ -0,0 +1,199 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * Authors:
+ *    James Henstridge <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or modify it under
+ * the terms of version 3 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "handler.h"
+#include <internal/safe_strerror.h>
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <QtConcurrent>
+#include <QFuture>
+#include <QFutureWatcher>
+#include <QDebug>
+#include <unity/util/ResourcePtr.h>
+
+using namespace unity::thumbnailer::internal;
+
+namespace {
+const char ART_ERROR[] = "com.canonical.Thumbnailer.Error.Failed";
+
+struct FdOrError {
+    QDBusUnixFileDescriptor fd;
+    QString error;
+};
+}
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct HandlerPrivate {
+    QDBusConnection bus;
+    const QDBusMessage message;
+
+    bool cancelled = false;
+    QFutureWatcher<FdOrError> checkWatcher;
+    QFutureWatcher<FdOrError> createWatcher;
+
+    HandlerPrivate(const QDBusConnection &bus, const QDBusMessage &message)
+        : bus(bus), message(message) {
+    }
+};
+
+Handler::Handler(const QDBusConnection &bus, const QDBusMessage &message)
+    : p(new HandlerPrivate(bus, message)) {
+    connect(&p->checkWatcher, &QFutureWatcher<FdOrError>::finished,
+            this, &Handler::checkFinished);
+    connect(&p->createWatcher, &QFutureWatcher<FdOrError>::finished,
+            this, &Handler::createFinished);
+}
+
+Handler::~Handler() {
+    p->cancelled = true;
+    // ensure that jobs occurring in the thread pool complete.
+    p->checkWatcher.waitForFinished();
+    p->createWatcher.waitForFinished();
+    qDebug() << "Handler" << this << "destroyed";
+}
+
+void Handler::begin() {
+    auto do_check = [this]() -> FdOrError {
+        try {
+            return FdOrError{check(), nullptr};
+        } catch (const std::exception &e) {
+            return FdOrError{QDBusUnixFileDescriptor(), e.what()};
+        }
+    };
+    p->checkWatcher.setFuture(QtConcurrent::run(do_check));
+}
+
+void Handler::checkFinished() {
+    if (p->cancelled)
+        return;
+
+    FdOrError fd_error;
+    try {
+        fd_error = p->checkWatcher.result();
+    } catch (const std::exception &e) {
+        sendError(e.what());
+        return;
+    }
+    if (!fd_error.error.isNull()) {
+        sendError(fd_error.error);
+        return;
+    }
+    // Did we find a valid thumbnail in the cache?
+    if (fd_error.fd.isValid()) {
+        sendThumbnail(fd_error.fd);
+    } else {
+        // otherwise move on to the download phase.
+        download();
+    }
+}
+
+void Handler::downloadFinished() {
+    if (p->cancelled)
+        return;
+
+    auto do_create = [this]() -> FdOrError {
+        try {
+            return FdOrError{create(), nullptr};
+        } catch (const std::exception &e) {
+            return FdOrError{QDBusUnixFileDescriptor(), e.what()};
+        }
+    };
+    p->createWatcher.setFuture(QtConcurrent::run(do_create));
+}
+
+void Handler::createFinished() {
+    if (p->cancelled)
+        return;
+
+    FdOrError fd_error;
+    try {
+        fd_error = p->createWatcher.result();
+    } catch (const std::exception &e) {
+        sendError(e.what());
+        return;
+    }
+    if (!fd_error.error.isEmpty()) {
+        sendError(fd_error.error);
+        return;
+    }
+    // Did we find a valid thumbnail in the cache?
+    if (fd_error.fd.isValid()) {
+        sendThumbnail(fd_error.fd);
+    } else {
+        // otherwise move on to the download phase.
+        download();
+    }
+}
+
+void Handler::sendThumbnail(const QDBusUnixFileDescriptor &unix_fd) {
+    p->bus.send(p->message.createReply(QVariant::fromValue(unix_fd)));
+    Q_EMIT finished();
+}
+
+void Handler::sendError(const QString &error) {
+    p->bus.send(p->message.createErrorReply(ART_ERROR, error));
+    Q_EMIT finished();
+}
+
+QDBusUnixFileDescriptor write_to_tmpfile(std::string const& image)
+{
+
+    typedef unity::util::ResourcePtr<int, decltype(&::close)> FdPtr;
+
+    static auto find_tmpdir = []
+    {
+        char const* dirp = getenv("TMPDIR");
+        std::string dir = dirp ? dirp : "/tmp";
+        return dir;
+    };
+    static std::string dir = find_tmpdir();
+
+    int fd = open(dir.c_str(), O_TMPFILE | O_RDWR);
+    if (fd < 0) {
+        std::string err = "cannot create tmpfile in " + dir + ": " + safe_strerror(errno);
+        throw std::runtime_error(err);
+    }
+    FdPtr fd_ptr(fd, ::close);
+    auto rc = write(fd_ptr.get(), &image[0], image.size());
+    if (rc == -1)
+    {
+        std::string err = "cannot write image data in " + dir + ": " + safe_strerror(errno);
+        throw std::runtime_error(err);
+    }
+    else if (std::string::size_type(rc) != image.size())
+    {
+        std::string err = "short write for image data in " + dir +
+            "(requested = " + std::to_string(image.size()) + ", actual = " + std::to_string(rc) + ")";
+        throw std::runtime_error(err);
+    }
+    lseek(fd, SEEK_SET, 0);  // No error check needed, can't fail.
+
+    return QDBusUnixFileDescriptor(fd_ptr.get());
+}
+
+}
+}
+}

=== added file 'src/service/handler.h'
--- src/service/handler.h	1970-01-01 00:00:00 +0000
+++ src/service/handler.h	2015-04-20 05:02:20 +0000
@@ -0,0 +1,89 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * Authors:
+ *    James Henstridge <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or modify it under
+ * the terms of version 3 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include <memory>
+#include <string>
+
+#include <QObject>
+#include <QDBusConnection>
+#include <QDBusMessage>
+#include <QDBusUnixFileDescriptor>
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct HandlerPrivate;
+
+class Handler : public QObject {
+    Q_OBJECT
+public:
+    Handler(const QDBusConnection &bus, const QDBusMessage &message);
+    ~Handler();
+
+    Handler(const Handler&) = delete;
+    Handler& operator=(Handler&) = delete;
+
+    void begin();
+
+protected:
+    void sendThumbnail(const QDBusUnixFileDescriptor &unix_fd);
+    void sendError(const QString &error);
+
+    // Methods to be overridden by handlers
+
+    // check() determines whether the requested thumbnail exists in
+    // the cache.  It is called synchronously in the thread pool.
+    //
+    // If it is available, it should be returned as a file descriptor,
+    // which will be returned to the user.
+    //
+    // If not, processing will continue.
+    virtual QDBusUnixFileDescriptor check() = 0;
+
+    // download() is expected to perform any asynchronous actions and
+    // end the request by either calling the downloadFinished() slot
+    // or sendError().
+    virtual void download() = 0;
+
+    // create() should create the thumbnail and store it in the cache,
+    // and return it as a file descriptor.  It is called synchronously
+    // in the thread pool.
+    virtual QDBusUnixFileDescriptor create() = 0;
+
+protected Q_SLOTS:
+    void checkFinished();
+    void downloadFinished();
+    void createFinished();
+
+Q_SIGNALS:
+    void finished();
+
+private:
+    std::unique_ptr<HandlerPrivate> p;
+};
+
+// Helper routine for creating an unnamed tmpfile from image data
+QDBusUnixFileDescriptor write_to_tmpfile(std::string const& image);
+
+}
+}
+}

=== modified file 'src/service/main.cpp'
--- src/service/main.cpp	2015-04-01 08:53:21 +0000
+++ src/service/main.cpp	2015-04-20 05:02:20 +0000
@@ -5,6 +5,8 @@
 #include "dbusinterface.h"
 #include "dbusinterfaceadaptor.h"
 
+using namespace unity::thumbnailer::service;
+
 static const char BUS_NAME[] = "com.canonical.Thumbnailer";
 static const char BUS_PATH[] = "/com/canonical/Thumbnailer";
 
@@ -13,9 +15,9 @@
 
     auto bus = QDBusConnection::sessionBus();
 
-    auto server = new DBusInterface();
-    new ThumbnailerAdaptor(server);
-    bus.registerObject(BUS_PATH, server);
+    unity::thumbnailer::service::DBusInterface server;
+    new ThumbnailerAdaptor(&server);
+    bus.registerObject(BUS_PATH, &server);
 
     if (!bus.registerService(BUS_NAME)) {
         fprintf(stderr, "Could no acquire D-Bus name %s.\n", BUS_NAME);

=== added file 'src/service/thumbnailhandler.cpp'
--- src/service/thumbnailhandler.cpp	1970-01-01 00:00:00 +0000
+++ src/service/thumbnailhandler.cpp	2015-04-20 05:02:20 +0000
@@ -0,0 +1,103 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * Authors:
+ *    James Henstridge <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or modify it under
+ * the terms of version 3 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "thumbnailhandler.h"
+#include <internal/safe_strerror.h>
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <unity/util/ResourcePtr.h>
+
+using namespace unity::thumbnailer::internal;
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct ThumbnailHandlerPrivate {
+    const std::shared_ptr<Thumbnailer> thumbnailer;
+    const QString filename;
+    const QDBusUnixFileDescriptor filename_fd;
+    const ThumbnailSize size;
+
+    ThumbnailHandlerPrivate(const std::shared_ptr<Thumbnailer> &thumbnailer,
+                            const QString &filename,
+                            const QDBusUnixFileDescriptor &filename_fd,
+                            ThumbnailSize size)
+        : thumbnailer(thumbnailer), filename(filename),
+          filename_fd(filename_fd), size(size) {
+    }
+};
+
+ThumbnailHandler::ThumbnailHandler(const QDBusConnection &bus,
+                                   const QDBusMessage &message,
+                                   const std::shared_ptr<Thumbnailer> &thumbnailer,
+                                   const QString &filename,
+                                   const QDBusUnixFileDescriptor &filename_fd,
+                                   ThumbnailSize size)
+    : Handler(bus, message), p(new ThumbnailHandlerPrivate(thumbnailer, filename, filename_fd, size)) {
+}
+
+ThumbnailHandler::~ThumbnailHandler() {
+}
+
+QDBusUnixFileDescriptor ThumbnailHandler::check() {
+    return QDBusUnixFileDescriptor();
+}
+
+void ThumbnailHandler::download() {
+    downloadFinished();
+}
+
+QDBusUnixFileDescriptor ThumbnailHandler::create() {
+    typedef unity::util::ResourcePtr<int, decltype(&::close)> FdPtr;
+    struct stat filename_stat, fd_stat;
+
+    if (stat(p->filename.toUtf8(), &filename_stat) < 0) {
+        throw std::runtime_error("Could not stat file");
+    }
+    if (fstat(p->filename_fd.fileDescriptor(), &fd_stat) < 0) {
+        throw std::runtime_error("Could not stat file descriptor");
+    }
+    if (filename_stat.st_dev != fd_stat.st_dev ||
+        filename_stat.st_ino != fd_stat.st_ino) {
+        throw std::runtime_error("filename refers to a different file to the file descriptor");
+    }
+
+    std::string art = p->thumbnailer->get_thumbnail(
+        p->filename.toStdString(), p->size, TN_REMOTE);
+
+    if (art.empty()) {
+        throw std::runtime_error("Could not get thumbnail");
+    }
+
+        // FIXME: check that the thumbnail was produced for fd_stat
+    FdPtr fd(open(art.c_str(), O_RDONLY), ::close);
+    if (fd.get() < 0) {
+        throw std::runtime_error(safe_strerror(errno));
+    }
+    return QDBusUnixFileDescriptor(fd.get());
+}
+
+}
+}
+}

=== added file 'src/service/thumbnailhandler.h'
--- src/service/thumbnailhandler.h	1970-01-01 00:00:00 +0000
+++ src/service/thumbnailhandler.h	2015-04-20 05:02:20 +0000
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * Authors:
+ *    James Henstridge <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or modify it under
+ * the terms of version 3 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "handler.h"
+#include <thumbnailer.h>
+
+#include <memory>
+
+#include <QObject>
+#include <QDBusConnection>
+#include <QDBusMessage>
+#include <QDBusUnixFileDescriptor>
+
+namespace unity {
+namespace thumbnailer {
+namespace service {
+
+struct ThumbnailHandlerPrivate;
+
+class ThumbnailHandler : public Handler {
+    Q_OBJECT
+public:
+    ThumbnailHandler(const QDBusConnection &bus, const QDBusMessage &message,
+                     const std::shared_ptr<Thumbnailer> &thumbnailer,
+                     const QString &filename,
+                     const QDBusUnixFileDescriptor &filename_fd,
+                     ThumbnailSize size);
+    ~ThumbnailHandler();
+
+protected:
+    virtual QDBusUnixFileDescriptor check() override;
+    virtual void download() override;
+    virtual QDBusUnixFileDescriptor create() override;
+
+private:
+    std::unique_ptr<ThumbnailHandlerPrivate> p;
+};
+
+}
+}
+}