Merge lp:~marcustomlinson/unity-scopes-api/lp-1325633 into lp:unity-scopes-api/devel

[Marcus Tomlinson]

Modified watch_event() method to be virtual rather than pure virtual as it is possible for this method to be called from watch_thread during destruction of DirWatcher.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:366
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/633/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-amd64-ci/187
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/185
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/185/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-i386-ci/185

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/633/rebuild

review: Approve (continuous-integration)

[Pete Woods (pete-woods)]

This looks like it'll fix the issue to me

review: Approve

[lp:~marcustomlinson/unity-scopes-api/lp-1325633]

367. By Marcus Tomlinson on 2014-06-03

Allow child class of DirWatcher (ScopesWatcher) to call cleanup() from destructor in order to join with watch_thread.

[Michal Hruby (mhr3)]

98 + // Close the file descriptor
99 + close(fd_);

This will be called twice now, perhaps move to DirWatcher's destructor only?

review: Needs Fixing

[lp:~marcustomlinson/unity-scopes-api/lp-1325633]

368. By Marcus Tomlinson on 2014-06-03

Moved close(fd_); to DirWatcher destructor

[Marcus Tomlinson (marcustomlinson)]

> 98 + // Close the file descriptor
> 99 + close(fd_);
>
> This will be called twice now, perhaps move to DirWatcher's destructor only?

Good catch! Thanks man.

[Michal Hruby (mhr3)]

/me happy

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:367
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/635/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-amd64-ci/189
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/187
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/187/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-i386-ci/187

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/635/rebuild

review: Approve (continuous-integration)

=== modified file 'scoperegistry/DirWatcher.cpp'
--- scoperegistry/DirWatcher.cpp	2014-05-27 12:46:25 +0000
+++ scoperegistry/DirWatcher.cpp	2014-06-03 09:07:37 +0000
@@ -45,44 +45,7 @@
 
 DirWatcher::~DirWatcher()
 {
-    {
-        std::lock_guard<std::mutex> lock(mutex_);
-
-        if (thread_state_ == Failed)
-        {
-            try
-            {
-                std::rethrow_exception(thread_exception_);
-            }
-            catch (std::exception const& e)
-            {
-                std::cerr << "~DirWatcher(): " << e.what() << std::endl;
-            }
-            catch (...)
-            {
-                std::cerr << "~DirWatcher(): watch_thread was aborted due to an unknown exception"
-                          << std::endl;
-            }
-        }
-        else
-        {
-            // Set state to Stopping
-            thread_state_ = Stopping;
-
-            // Remove watches (causes read to return)
-            for (auto& wd : wds_)
-            {
-                inotify_rm_watch(fd_, wd.first);
-            }
-            wds_.clear();
-        }
-    }
-
-    // Wait for thread to terminate
-    if (thread_.joinable())
-    {
-        thread_.join();
-    }
+    cleanup();
 
     // Close the file descriptor
     close(fd_);
@@ -152,6 +115,48 @@
     }
 }
 
+void DirWatcher::cleanup()
+{
+    {
+        std::lock_guard<std::mutex> lock(mutex_);
+
+        if (thread_state_ == Failed)
+        {
+            try
+            {
+                std::rethrow_exception(thread_exception_);
+            }
+            catch (std::exception const& e)
+            {
+                std::cerr << "~DirWatcher(): " << e.what() << std::endl;
+            }
+            catch (...)
+            {
+                std::cerr << "~DirWatcher(): watch_thread was aborted due to an unknown exception"
+                          << std::endl;
+            }
+        }
+        else
+        {
+            // Set state to Stopping
+            thread_state_ = Stopping;
+
+            // Remove watches (causes read to return)
+            for (auto& wd : wds_)
+            {
+                inotify_rm_watch(fd_, wd.first);
+            }
+            wds_.clear();
+        }
+    }
+
+    // Wait for thread to terminate
+    if (thread_.joinable())
+    {
+        thread_.join();
+    }
+}
+
 void DirWatcher::watch_thread()
 {
     try

=== modified file 'scoperegistry/DirWatcher.h'
--- scoperegistry/DirWatcher.h	2014-05-22 05:57:31 +0000
+++ scoperegistry/DirWatcher.h	2014-06-03 09:07:37 +0000
@@ -48,11 +48,14 @@
     };
 
     DirWatcher();
-    ~DirWatcher();
+    virtual ~DirWatcher();
 
     void add_watch(std::string const& path);
     void remove_watch(std::string const& path);
 
+protected:
+    void cleanup();
+
 private:
     enum ThreadState
     {
@@ -71,7 +74,7 @@
     std::exception_ptr thread_exception_;
 
     void watch_thread();
-    virtual void watch_event(EventType event_type, FileType file_type, std::string const& path) = 0;
+    virtual void watch_event(EventType, FileType, std::string const&) {}
 };
 
 } // namespace scoperegistry

=== modified file 'scoperegistry/ScopesWatcher.cpp'
--- scoperegistry/ScopesWatcher.cpp	2014-05-27 09:54:38 +0000
+++ scoperegistry/ScopesWatcher.cpp	2014-06-03 09:07:37 +0000
@@ -35,6 +35,11 @@
 {
 }
 
+ScopesWatcher::~ScopesWatcher()
+{
+    cleanup();
+}
+
 void ScopesWatcher::add_install_dir(std::string const& dir)
 {
     try

=== modified file 'scoperegistry/ScopesWatcher.h'
--- scoperegistry/ScopesWatcher.h	2014-05-27 09:54:38 +0000
+++ scoperegistry/ScopesWatcher.h	2014-06-03 09:07:37 +0000
@@ -36,6 +36,8 @@
     ScopesWatcher(unity::scopes::internal::RegistryObject::SPtr registry,
                   std::function<void(std::pair<std::string, std::string> const&)> ini_added_callback);
 
+    ~ScopesWatcher();
+
     void add_install_dir(std::string const& dir);
 
 private: