Merge lp:~jamesh/mediascanner2/bug-1489656 into lp:mediascanner2

[James Henstridge]

When multiple volumes are mounted in quick succession, scan them serially to avoid reentrancy problems in the initial scan.

Queue up volume mount/unmount events so they are processed serially rather than all at once.

[unity-api-1-bot (unity-api-1-bot)]

FAILED: Continuous integration, rev:339
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/mediascanner2/bug-1489656/+merge/304350/+edit-commit-message

https://jenkins.canonical.com/unity-api-1/job/lp-mediascanner2-ci/5/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/527/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/533
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/435
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial+overlay/435
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/435
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/365
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/365/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/365
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/365/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/365
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/365/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/365
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/365/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/365
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/365/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/365/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/365
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/365/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/365
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/365/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/365
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/365/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-mediascanner2-ci/5/rebuild

review: Needs Fixing (continuous-integration)

[Michi Henning (michihenning)]

From reading the code, this looks nice to me. I'm not familiar with the code though. Approving seeing that there is no other suitable candidate.

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2016-02-25 01:53:30 +0000
+++ CMakeLists.txt	2016-08-31 03:52:20 +0000
@@ -1,7 +1,7 @@
+cmake_minimum_required(VERSION 3.0.2)
 project(mediascanner2 CXX C)
-cmake_minimum_required(VERSION 2.8.9)
 
-set(MEDIASCANNER_VERSION "0.111")
+set(MEDIASCANNER_VERSION "0.112")
 
 execute_process(
   COMMAND /bin/sh ${CMAKE_CURRENT_SOURCE_DIR}/get-soversion.sh
@@ -93,6 +93,7 @@
     test_sqliteutils
     test_mfbuilder
     test_subtreewatcher
+    test_volumemanager
     test_dbus
     test_qml
     test_util

=== modified file 'debian/changelog'
--- debian/changelog	2016-05-26 06:02:56 +0000
+++ debian/changelog	2016-08-31 03:52:20 +0000
@@ -1,3 +1,11 @@
+mediascanner2 (0.112-0ubuntu1) UNRELEASED; urgency=medium
+
+  * When multiple volumes are mounted in quick succession, scan them
+    serially to avoid reentrancy problems in the initial scan. (LP:
+    #1489656)
+
+ -- James Henstridge <[e-mail]>  Wed, 31 Aug 2016 11:26:18 +0800
+
 mediascanner2 (0.111+16.10.20160526-0ubuntu1) yakkety; urgency=medium
 
   [ Alex Tu ]

=== modified file 'src/daemon/CMakeLists.txt'
--- src/daemon/CMakeLists.txt	2015-11-02 09:38:48 +0000
+++ src/daemon/CMakeLists.txt	2016-08-31 03:52:20 +0000
@@ -4,6 +4,7 @@
 add_library(scannerstuff STATIC
   InvalidationSender.cc
   MountWatcher.cc
+  VolumeManager.cc
   SubtreeWatcher.cc
   Scanner.cc
   ../mediascanner/utils.cc

=== modified file 'src/daemon/InvalidationSender.cc'
--- src/daemon/InvalidationSender.cc	2016-03-16 07:13:27 +0000
+++ src/daemon/InvalidationSender.cc	2016-08-31 03:52:20 +0000
@@ -31,6 +31,8 @@
 static const char SCOPES_DBUS_PATH[] = "/com/canonical/unity/scopes";
 static const char SCOPES_INVALIDATE_RESULTS[] = "InvalidateResults";
 
+namespace mediascanner {
+
 InvalidationSender::InvalidationSender() :
     bus(nullptr, g_object_unref) {
 }
@@ -87,3 +89,5 @@
     invalidator->timeout_id = 0;
     return G_SOURCE_REMOVE;
 }
+
+}

=== modified file 'src/daemon/InvalidationSender.hh'
--- src/daemon/InvalidationSender.hh	2016-03-16 07:13:27 +0000
+++ src/daemon/InvalidationSender.hh	2016-08-31 03:52:20 +0000
@@ -24,6 +24,8 @@
 
 typedef struct _GDBusConnection GDBusConnection;
 
+namespace mediascanner {
+
 /**
  * A class that sends a broadcast signal that the state of media
  * files has changed.
@@ -48,4 +50,6 @@
     int delay = 0;
 };
 
+}
+
 #endif

=== modified file 'src/daemon/SubtreeWatcher.hh'
--- src/daemon/SubtreeWatcher.hh	2016-02-09 03:36:14 +0000
+++ src/daemon/SubtreeWatcher.hh	2016-08-31 03:52:20 +0000
@@ -22,10 +22,9 @@
 
 #include<string>
 
+namespace mediascanner {
+
 class InvalidationSender;
-
-namespace mediascanner {
-
 class MediaStore;
 class MetadataExtractor;
 

=== added file 'src/daemon/VolumeManager.cc'
--- src/daemon/VolumeManager.cc	1970-01-01 00:00:00 +0000
+++ src/daemon/VolumeManager.cc	2016-08-31 03:52:20 +0000
@@ -0,0 +1,232 @@
+/*
+ * Copyright (C) 2016 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 "VolumeManager.hh"
+
+#include <mediascanner/MediaFile.hh>
+#include <mediascanner/MediaStore.hh>
+#include <extractor/DetectedFile.hh>
+#include <extractor/MetadataExtractor.hh>
+#include "InvalidationSender.hh"
+#include "Scanner.hh"
+#include "SubtreeWatcher.hh"
+#include "../mediascanner/internal/utils.hh"
+
+#include <glib.h>
+
+#include <cassert>
+#include <cstdio>
+#include <map>
+#include <deque>
+
+using namespace std;
+
+namespace {
+
+enum class VolumeEventType {
+    added,
+    removed,
+};
+
+struct VolumeEvent {
+    VolumeEventType type;
+    string path;
+
+    VolumeEvent(VolumeEventType type, const string& path)
+        : type(type), path(path) {}
+};
+
+}
+
+namespace mediascanner {
+
+struct VolumeManagerPrivate {
+    MediaStore& store;
+    MetadataExtractor& extractor;
+    InvalidationSender& invalidator;
+
+    map<string, unique_ptr<SubtreeWatcher>> volumes;
+    deque<VolumeEvent> pending;
+    unsigned int idle_id = 0;
+
+    VolumeManagerPrivate(MediaStore& store, MetadataExtractor& extractor,
+                         InvalidationSender& invalidator);
+    ~VolumeManagerPrivate();
+
+    void queueUpdate(VolumeEventType type, const string& path);
+    static gboolean processEvent(void *user_data) noexcept;
+
+    void addVolume(const string& path);
+    void removeVolume(const string& path);
+    void readFiles(const string& subdir, const MediaType type);
+};
+
+VolumeManager::VolumeManager(MediaStore& store, MetadataExtractor& extractor,
+                             InvalidationSender& invalidator)
+    : p(new VolumeManagerPrivate(store, extractor, invalidator)) {
+}
+
+VolumeManager::~VolumeManager() = default;
+
+void VolumeManager::queueAddVolume(const string& path) {
+    p->queueUpdate(VolumeEventType::added, path);
+}
+
+void VolumeManager::queueRemoveVolume(const string& path) {
+    p->queueUpdate(VolumeEventType::removed, path);
+}
+
+bool VolumeManager::idle() const {
+    // idle_id will only be reset once the scanning job has completed.
+    return p->idle_id == 0;
+}
+
+VolumeManagerPrivate::VolumeManagerPrivate(MediaStore& store,
+                                           MetadataExtractor& extractor,
+                                           InvalidationSender& invalidator)
+    : store(store), extractor(extractor), invalidator(invalidator) {
+}
+
+VolumeManagerPrivate::~VolumeManagerPrivate()
+{
+    if (idle_id != 0) {
+        g_source_remove(idle_id);
+    }
+}
+
+void VolumeManagerPrivate::queueUpdate(VolumeEventType type,
+                                       const string& path) {
+    for (auto it = pending.begin(); it != pending.end();) {
+        if (it->path == path) {
+            it = pending.erase(it);
+        } else {
+            ++it;
+        }
+    }
+    pending.emplace_back(type, path);
+    if (idle_id == 0) {
+        idle_id = g_idle_add(&VolumeManagerPrivate::processEvent, this);
+    }
+}
+
+gboolean VolumeManagerPrivate::processEvent(void *user_data) noexcept {
+    auto *p = reinterpret_cast<VolumeManagerPrivate*>(user_data);
+
+    while (!p->pending.empty()) {
+        auto event = move(p->pending.front());
+        p->pending.pop_front();
+
+        switch (event.type) {
+        case VolumeEventType::added:
+            p->addVolume(event.path);
+            break;
+        case VolumeEventType::removed:
+            p->removeVolume(event.path);
+            break;
+        }
+    }
+    p->invalidator.invalidate();
+    p->idle_id = 0;
+    return G_SOURCE_REMOVE;
+}
+
+void VolumeManagerPrivate::addVolume(const string& path) {
+    assert(path[0] == '/');
+    if(volumes.find(path) != volumes.end()) {
+        return;
+    }
+    if(is_rootlike(path)) {
+        fprintf(stderr, "Directory %s looks like a top level root directory, skipping it (%s).\n",
+                path.c_str(), __PRETTY_FUNCTION__);
+        return;
+    }
+    if(is_optical_disc(path)) {
+        fprintf(stderr, "Directory %s looks like an optical disc, skipping it.\n", path.c_str());
+        return;
+    }
+    if(has_scanblock(path)) {
+        fprintf(stderr, "Directory %s has a scan block file, skipping it.\n", path.c_str());
+        return;
+    }
+    unique_ptr<SubtreeWatcher> sw(new SubtreeWatcher(store, extractor, invalidator));
+    store.restoreItems(path);
+    store.pruneDeleted();
+    readFiles(path, AllMedia);
+    sw->addDir(path);
+    volumes[path] = move(sw);
+}
+
+void VolumeManagerPrivate::removeVolume(const string& path) {
+    assert(path[0] == '/');
+    if(volumes.find(path) == volumes.end())
+        return;
+    store.archiveItems(path);
+    volumes.erase(path);
+}
+
+void VolumeManagerPrivate::readFiles(const string &subdir, const MediaType type) {
+    Scanner s(&extractor, subdir, type);
+    MediaStoreTransaction txn = store.beginTransaction();
+    const int update_interval = 10; // How often to send invalidations.
+    struct timespec previous_update, current_time;
+    clock_gettime(CLOCK_MONOTONIC, &previous_update);
+    previous_update.tv_sec -= update_interval/2; // Send the first update sooner for better visual appeal.
+    while(true) {
+        try {
+            auto d = s.next();
+            clock_gettime(CLOCK_MONOTONIC, &current_time);
+            while(g_main_context_pending(g_main_context_default())) {
+                g_main_context_iteration(g_main_context_default(), FALSE);
+            }
+            if(current_time.tv_sec - previous_update.tv_sec >= update_interval) {
+                txn.commit();
+                invalidator.invalidate();
+                previous_update = current_time;
+            }
+            // If the file is broken or unchanged, use fallback.
+            if (store.is_broken_file(d.filename, d.etag)) {
+                fprintf(stderr, "Using fallback data for unscannable file %s.\n", d.filename.c_str());
+                store.insert(extractor.fallback_extract(d));
+                continue;
+            }
+            if(d.etag == store.getETag(d.filename))
+                continue;
+
+            try {
+                store.insert_broken_file(d.filename, d.etag);
+                MediaFile media;
+                try {
+                    media = extractor.extract(d);
+                } catch (const runtime_error &e) {
+                    fprintf(stderr, "Error extracting from '%s': %s\n",
+                            d.filename.c_str(), e.what());
+                    media = extractor.fallback_extract(d);
+                }
+                store.insert(std::move(media));
+            } catch(const exception &e) {
+                fprintf(stderr, "Error when indexing: %s\n", e.what());
+            }
+        } catch(const StopIteration &stop) {
+            break;
+        }
+    }
+    txn.commit();
+}
+
+}

=== added file 'src/daemon/VolumeManager.hh'
--- src/daemon/VolumeManager.hh	1970-01-01 00:00:00 +0000
+++ src/daemon/VolumeManager.hh	2016-08-31 03:52:20 +0000
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2016 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>
+
+namespace mediascanner {
+
+class MediaStore;
+class MetadataExtractor;
+class InvalidationSender;
+
+struct VolumeManagerPrivate;
+
+class VolumeManager final {
+public:
+    VolumeManager(MediaStore& store, MetadataExtractor& extractor, InvalidationSender& invalidator);
+    ~VolumeManager();
+    VolumeManager(const VolumeManager&) = delete;
+    VolumeManager& operator=(const VolumeManager&) = delete;
+
+    void queueAddVolume(const std::string& path);
+    void queueRemoveVolume(const std::string& path);
+
+    bool idle() const;
+
+private:
+    std::unique_ptr<VolumeManagerPrivate> p;
+};
+
+}

=== modified file 'src/daemon/scannerdaemon.cc'
--- src/daemon/scannerdaemon.cc	2016-03-17 02:11:17 +0000
+++ src/daemon/scannerdaemon.cc	2016-08-31 03:52:20 +0000
@@ -32,13 +32,10 @@
 
 #include "../mediascanner/MediaFile.hh"
 #include "../mediascanner/MediaStore.hh"
-#include "../extractor/DetectedFile.hh"
 #include "../extractor/MetadataExtractor.hh"
 #include "MountWatcher.hh"
-#include "SubtreeWatcher.hh"
-#include "Scanner.hh"
 #include "InvalidationSender.hh"
-#include "../mediascanner/internal/utils.hh"
+#include "VolumeManager.hh"
 
 using namespace std;
 
@@ -74,9 +71,6 @@
     void setupBus();
     void setupSignals();
     void setupMountWatcher();
-    void readFiles(MediaStore &store, const string &subdir, const MediaType type);
-    void addDir(const string &dir);
-    void removeDir(const string &dir);
     static gboolean signalCallback(gpointer data);
     static void busNameLostCallback(GDBusConnection *connection, const char *name, gpointer data);
     void mountEvent(const MountWatcher::Info &info);
@@ -86,8 +80,8 @@
     string cachedir;
     unique_ptr<MediaStore> store;
     unique_ptr<MetadataExtractor> extractor;
-    map<string, unique_ptr<SubtreeWatcher>> subtrees;
     InvalidationSender invalidator;
+    unique_ptr<VolumeManager> volumes;
     unique_ptr<GMainLoop,void(*)(GMainLoop*)> main_loop;
     unique_ptr<GDBusConnection,void(*)(void*)> session_bus;
     unsigned int bus_name_id = 0;
@@ -99,6 +93,7 @@
     setupBus();
     store.reset(new MediaStore(MS_READ_WRITE, "/media/"));
     extractor.reset(new MetadataExtractor(session_bus.get()));
+    volumes.reset(new VolumeManager(*store, *extractor, invalidator));
 
     setupMountWatcher();
 
@@ -111,13 +106,13 @@
     // it falls back to home directory. This would mean scanning the entire home
     // directory. This is probably not what people want so skip if this is the case.
     if (musicdir && !is_same_directory(musicdir, homedir))
-        addDir(musicdir);
+        volumes->queueAddVolume(musicdir);
 
     if (videodir && !is_same_directory(videodir, homedir))
-        addDir(videodir);
+        volumes->queueAddVolume(videodir);
 
     if (picturesdir && !is_same_directory(picturesdir, homedir))
-        addDir(picturesdir);
+        volumes->queueAddVolume(picturesdir);
 
     // In case someone opened the db file before we could populate it.
     invalidator.invalidate();
@@ -178,89 +173,6 @@
     sigterm_id = g_unix_signal_add(SIGTERM, &ScannerDaemon::signalCallback, this);
 }
 
-void ScannerDaemon::addDir(const string &dir) {
-    assert(dir[0] == '/');
-    if(subtrees.find(dir) != subtrees.end()) {
-        return;
-    }
-    if(is_rootlike(dir)) {
-        fprintf(stderr, "Directory %s looks like a top level root directory, skipping it (%s).\n",
-                dir.c_str(), __PRETTY_FUNCTION__);
-        return;
-    }
-    if(is_optical_disc(dir)) {
-        fprintf(stderr, "Directory %s looks like an optical disc, skipping it.\n", dir.c_str());
-        return;
-    }
-    if(has_scanblock(dir)) {
-        fprintf(stderr, "Directory %s has a scan block file, skipping it.\n", dir.c_str());
-        return;
-    }
-    unique_ptr<SubtreeWatcher> sw(new SubtreeWatcher(*store.get(), *extractor.get(), invalidator));
-    store->restoreItems(dir);
-    store->pruneDeleted();
-    readFiles(*store.get(), dir, AllMedia);
-    sw->addDir(dir);
-    subtrees[dir] = move(sw);
-}
-
-void ScannerDaemon::removeDir(const string &dir) {
-    assert(dir[0] == '/');
-    if(subtrees.find(dir) == subtrees.end())
-        return;
-    store->archiveItems(dir);
-    subtrees.erase(dir);
-}
-
-void ScannerDaemon::readFiles(MediaStore &store, const string &subdir, const MediaType type) {
-    Scanner s(extractor.get(), subdir, type);
-    MediaStoreTransaction txn = store.beginTransaction();
-    const int update_interval = 10; // How often to send invalidations.
-    struct timespec previous_update, current_time;
-    clock_gettime(CLOCK_MONOTONIC, &previous_update);
-    previous_update.tv_sec -= update_interval/2; // Send the first update sooner for better visual appeal.
-    while(true) {
-        try {
-            auto d = s.next();
-            clock_gettime(CLOCK_MONOTONIC, &current_time);
-            while(g_main_context_pending(g_main_context_default())) {
-                g_main_context_iteration(g_main_context_default(), FALSE);
-            }
-            if(current_time.tv_sec - previous_update.tv_sec >= update_interval) {
-                txn.commit();
-                invalidator.invalidate();
-                previous_update = current_time;
-            }
-            // If the file is broken or unchanged, use fallback.
-            if (store.is_broken_file(d.filename, d.etag)) {
-                fprintf(stderr, "Using fallback data for unscannable file %s.\n", d.filename.c_str());
-                store.insert(extractor->fallback_extract(d));
-                continue;
-            }
-            if(d.etag == store.getETag(d.filename))
-                continue;
-
-            try {
-                store.insert_broken_file(d.filename, d.etag);
-                MediaFile media;
-                try {
-                    media = extractor->extract(d);
-                } catch (const runtime_error &e) {
-                    fprintf(stderr, "Error extracting from '%s': %s\n",
-                            d.filename.c_str(), e.what());
-                    media = extractor->fallback_extract(d);
-                }
-                store.insert(std::move(media));
-            } catch(const exception &e) {
-                fprintf(stderr, "Error when indexing: %s\n", e.what());
-            }
-        } catch(const StopIteration &stop) {
-            break;
-        }
-    }
-    txn.commit();
-}
-
 int ScannerDaemon::run() {
     g_main_loop_run(main_loop.get());
     return 99;
@@ -279,25 +191,14 @@
 }
 
 void ScannerDaemon::mountEvent(const MountWatcher::Info& info) {
-    bool changed = false;
     if (info.is_mounted) {
         printf("Volume %s was mounted.\n", info.mount_point.c_str());
         if (info.mount_point.substr(0, 6) == "/media") {
-            addDir(info.mount_point);
-            changed = true;
+            volumes->queueAddVolume(info.mount_point);
         }
     } else {
         printf("Volume %s was unmounted.\n", info.mount_point.c_str());
-        if (subtrees.find(info.mount_point) != subtrees.end()) {
-            removeDir(info.mount_point);
-            changed = true;
-        } else {
-            // This volume was not tracked because it looked rootlike.
-            // Thus we don't need to do anything.
-        }
-    }
-    if (changed) {
-        invalidator.invalidate();
+        volumes->queueRemoveVolume(info.mount_point);
     }
 }
 

=== modified file 'test/CMakeLists.txt'
--- test/CMakeLists.txt	2016-02-25 01:53:30 +0000
+++ test/CMakeLists.txt	2016-08-31 03:52:20 +0000
@@ -60,6 +60,13 @@
 set_tests_properties(test_subtreewatcher PROPERTIES
   ENVIRONMENT "GIO_MODULE_DIR=${CMAKE_CURRENT_BINARY_DIR}/modules")
 
+add_executable(test_volumemanager test_volumemanager.cc)
+target_link_libraries(test_volumemanager mediascanner scannerstuff gtest)
+add_test(test_volumemanager test_volumemanager)
+# The gvfs modules interfere with the private D-Bus test fixtures
+set_tests_properties(test_volumemanager PROPERTIES
+  ENVIRONMENT "GIO_MODULE_DIR=${CMAKE_CURRENT_BINARY_DIR}/modules")
+
 add_executable(test_sqliteutils test_sqliteutils.cc)
 target_link_libraries(test_sqliteutils gtest ${MEDIASCANNER_DEPS_LDFLAGS})
 add_test(test_sqliteutils test_sqliteutils)

=== added file 'test/test_volumemanager.cc'
--- test/test_volumemanager.cc	1970-01-01 00:00:00 +0000
+++ test/test_volumemanager.cc	2016-08-31 03:52:20 +0000
@@ -0,0 +1,234 @@
+/*
+ * Copyright (C) 2016 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 <mediascanner/MediaFile.hh>
+#include <mediascanner/MediaStore.hh>
+#include <extractor/MetadataExtractor.hh>
+#include <daemon/InvalidationSender.hh>
+#include <daemon/VolumeManager.hh>
+
+#include "test_config.h"
+
+#include <fcntl.h>
+#include <algorithm>
+#include <cstdio>
+#include <memory>
+#include <stdexcept>
+#include <string>
+
+#include <gio/gio.h>
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace mediascanner;
+
+namespace {
+
+typedef std::unique_ptr<GDBusConnection, decltype(&g_object_unref)> GDBusConnectionPtr;
+
+void copy_file(const string &src, const string &dst) {
+    FILE* f = fopen(src.c_str(), "r");
+    ASSERT_TRUE(f);
+    fseek(f, 0, SEEK_END);
+    size_t size = ftell(f);
+
+    char* buf = new char[size];
+
+    fseek(f, 0, SEEK_SET);
+    ASSERT_EQ(fread(buf, 1, size, f), size);
+    fclose(f);
+
+    f = fopen(dst.c_str(), "w");
+    ASSERT_TRUE(f);
+    ASSERT_EQ(fwrite(buf, 1, size, f), size);
+    fclose(f);
+    delete[] buf;
+}
+
+}
+
+class VolumeManagerTest : public ::testing::Test {
+protected:
+    virtual void SetUp() override {
+        main_loop_.reset(g_main_loop_new(nullptr, false));
+
+        tmpdir_ = TEST_DIR "/volumemanager-test.XXXXXX";
+        ASSERT_NE(nullptr, mkdtemp(&tmpdir_[0]));
+
+        test_dbus_.reset(g_test_dbus_new(G_TEST_DBUS_NONE));
+        g_test_dbus_add_service_dir(test_dbus_.get(), TEST_DIR "/services");
+        g_test_dbus_up(test_dbus_.get());
+        session_bus_ = make_connection();
+
+        store_.reset(new MediaStore(":memory:", MS_READ_WRITE));
+        extractor_.reset(new MetadataExtractor(session_bus_.get()));
+        invalidator_.reset(new InvalidationSender);
+        volumes_.reset(new VolumeManager(*store_, *extractor_, *invalidator_));
+    }
+
+    virtual void TearDown() override {
+        volumes_.reset();
+        invalidator_.reset();
+        extractor_.reset();
+        store_.reset();
+
+        session_bus_.reset();
+        test_dbus_.reset();
+
+        if (!tmpdir_.empty()) {
+            string cmd = "rm -rf " + tmpdir_;
+            ASSERT_EQ(0, system(cmd.c_str()));
+        }
+    }
+
+    GDBusConnectionPtr make_connection() {
+        GError *error = nullptr;
+        char *address = g_dbus_address_get_for_bus_sync(G_BUS_TYPE_SESSION, nullptr, &error);
+        if (!address) {
+            string errortxt(error->message);
+            g_error_free(error);
+            throw std::runtime_error(
+                string("Failed to determine session bus address: ") + errortxt);
+        }
+        GDBusConnectionPtr bus(
+            g_dbus_connection_new_for_address_sync(
+                address, static_cast<GDBusConnectionFlags>(G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT | G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION), nullptr, nullptr, &error),
+            g_object_unref);
+        g_free(address);
+        if (!bus) {
+            string errortxt(error->message);
+            g_error_free(error);
+            throw std::runtime_error(
+                string("Failed to connect to session bus: ") + errortxt);
+        }
+        return std::move(bus);
+    }
+
+    void wait_until_idle() {
+        while (g_main_context_iteration(nullptr, FALSE)) {
+            if (volumes_->idle()) {
+                break;
+            }
+        }
+    }
+
+    string tmpdir_;
+    unique_ptr<GTestDBus,decltype(&g_object_unref)> test_dbus_ {nullptr, g_object_unref};
+    unique_ptr<MediaStore> store_;
+    unique_ptr<VolumeManager> volumes_;
+
+private:
+    GDBusConnectionPtr session_bus_ {nullptr, g_object_unref};
+    unique_ptr<MetadataExtractor> extractor_;
+    unique_ptr<InvalidationSender> invalidator_;
+
+    unique_ptr<GMainLoop, decltype(&g_main_loop_unref)> main_loop_ {nullptr, g_main_loop_unref};
+};
+
+TEST_F(VolumeManagerTest, add_remove_volumes)
+{
+    const string volume1 = tmpdir_ + "/volume1";
+    const string volume2 = tmpdir_ + "/volume2";
+    ASSERT_EQ(0, mkdir(volume1.c_str(), 0755));
+    ASSERT_EQ(0, mkdir(volume2.c_str(), 0755));
+
+    const string file1 = volume1 + "/file1.ogg";
+    const string file2 = volume1 + "/file2.ogg";
+    const string file3 = volume2 + "/file3.ogg";
+
+    copy_file(SOURCE_DIR "/media/testfile.ogg", file1);
+    copy_file(SOURCE_DIR "/media/testfile.ogg", file2);
+    copy_file(SOURCE_DIR "/media/testfile.ogg", file3);
+
+    volumes_->queueAddVolume(volume1);
+    wait_until_idle();
+    EXPECT_EQ(store_->size(), 2);
+    store_->lookup(file1);
+    store_->lookup(file2);
+    try {
+        store_->lookup(file3);
+        FAIL();
+    } catch (const runtime_error&) {
+    }
+
+    // Queue up two operations
+    volumes_->queueAddVolume(volume2);
+    volumes_->queueRemoveVolume(volume1);
+    wait_until_idle();
+    EXPECT_EQ(store_->size(), 1);
+    store_->lookup(file3);
+    try {
+        store_->lookup(file1);
+        store_->lookup(file2);
+    } catch (const runtime_error&) {
+    }
+
+    volumes_->queueRemoveVolume(volume2);
+    wait_until_idle();
+    EXPECT_EQ(store_->size(), 0);
+
+    // This should result in only volume2 being added
+    volumes_->queueAddVolume(volume1);
+    volumes_->queueAddVolume(volume2);
+    volumes_->queueRemoveVolume(volume1);
+    wait_until_idle();
+    EXPECT_EQ(store_->size(), 1);
+    store_->lookup(file3);
+}
+
+TEST_F(VolumeManagerTest, add_volume_during_initial_scan)
+{
+    const string volume1 = tmpdir_ + "/volume1";
+    const string volume2 = tmpdir_ + "/volume2";
+    ASSERT_EQ(0, mkdir(volume1.c_str(), 0755));
+    ASSERT_EQ(0, mkdir(volume2.c_str(), 0755));
+
+    const string file1 = volume1 + "/file1.ogg";
+    const string file2 = volume1 + "/file2.ogg";
+    const string file3 = volume2 + "/file3.ogg";
+
+    copy_file(SOURCE_DIR "/media/testfile.ogg", file1);
+    copy_file(SOURCE_DIR "/media/testfile.ogg", file2);
+    copy_file(SOURCE_DIR "/media/testfile.ogg", file3);
+
+    // Set an idle function that should be called during the initial
+    // scan that will queue up adding a second volume.
+    function<void()> callback = [&] {
+        // We expect that this is called during the scan
+        EXPECT_FALSE(volumes_->idle());
+        volumes_->queueAddVolume(volume2);
+    };
+    g_idle_add([](void *user_data) -> gboolean {
+            auto callback = *reinterpret_cast<function<void()>*>(user_data);
+            callback();
+            return G_SOURCE_REMOVE;
+        }, &callback);
+
+    volumes_->queueAddVolume(volume1);
+    wait_until_idle();
+    EXPECT_EQ(store_->size(), 3);
+    store_->lookup(file1);
+    store_->lookup(file2);
+    store_->lookup(file3);
+}
+
+int main(int argc, char **argv) {
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}