Merge lp:~michihenning/persistent-cache-cpp/fix-move-construct into lp:persistent-cache-cpp/devel

[Michi Henning]

Fixed assertion failure when move-constructing from internal stats instance.

Fixed assertion failure when move-constructing from internal stats instance.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:39
http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-ci/25/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-wily-amd64-ci/25
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-wily-armhf-ci/25
        deb: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-wily-armhf-ci/25/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-wily-i386-ci/25

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/persistent-cache-cpp-devel-ci/25/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:40
http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-ci/26/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-wily-amd64-ci/26
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-wily-armhf-ci/26
        deb: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-wily-armhf-ci/26/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-wily-i386-ci/26

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/persistent-cache-cpp-devel-ci/26/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:40
http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-ci/29/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-xenial-amd64-ci/3
    FAILURE: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-xenial-armhf-ci/3/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-xenial-i386-ci/3/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/persistent-cache-cpp-devel-ci/29/rebuild

review: Needs Fixing (continuous-integration)

[lp:~michihenning/persistent-cache-cpp/fix-move-construct]

41. By Michi Henning on 2015-12-10

Merged dependent branch.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:41
http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-ci/30/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-xenial-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-xenial-armhf-ci/4
        deb: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-xenial-armhf-ci/4/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/persistent-cache-cpp-devel-xenial-i386-ci/4

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/persistent-cache-cpp-devel-ci/30/rebuild

review: Approve (continuous-integration)

[James Henstridge (jamesh)]

Looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'debian/changelog'
--- debian/changelog	2015-08-19 21:50:26 +0000
+++ debian/changelog	2015-12-10 06:07:16 +0000
@@ -1,3 +1,10 @@
+persistent-cache-cpp (1.0.4+15.10.20150819.2-0ubuntu2) UNRELEASED; urgency=medium
+
+  * Fixed assertion failure when move-constructing a stats instance from
+    the internal instance.
+
+ -- Michi Henning <[e-mail]>  Wed, 09 Dec 2015 16:16:41 +1000
+
 persistent-cache-cpp (1.0.4+15.10.20150819.2-0ubuntu1) wily; urgency=medium
 
   [ Michi Henning ]

=== modified file 'include/core/persistent_cache_stats.h'
--- include/core/persistent_cache_stats.h	2015-08-19 04:44:35 +0000
+++ include/core/persistent_cache_stats.h	2015-12-10 06:07:16 +0000
@@ -236,7 +236,7 @@
     // We store a shared_ptr for efficiency. When the caller
     // retrieves the stats, we set p_ to point at the PersistentStringCacheStats
     // inside the cache. If the caller makes a copy or assigns,
-    // We create a new instance, to provide value semantics. This means
+    // we create a new instance, to provide value semantics. This means
     // that we don't have to copy all of the stats each time the caller
     // gets them.
     std::shared_ptr<internal::PersistentStringCacheStats const> p_;

=== modified file 'src/core/persistent_cache_stats.cpp'
--- src/core/persistent_cache_stats.cpp	2015-08-19 05:31:09 +0000
+++ src/core/persistent_cache_stats.cpp	2015-12-10 06:07:16 +0000
@@ -60,11 +60,15 @@
 PersistentCacheStats::PersistentCacheStats(PersistentCacheStats&& other) noexcept
     : internal_(false)
 {
-    // The cache always passes the internal instance to the event
-    // handler by const ref, so it is not possible to move construct from it.
-    assert(!other.internal_);
-
-    p_ = other.p_;  // Move must leave the instance in a usable state, so we just copy the shared_ptr.
+    if (other.internal_)
+    {
+        // Source is pointing at the internal instance, so we make a copy here to de-couple from it.
+        p_ = make_shared<internal::PersistentStringCacheStats>(*other.p_);
+    }
+    else
+    {
+        p_ = other.p_;  // Move must leave the instance in a usable state, so we just copy the shared_ptr.
+    }
 }
 
 PersistentCacheStats& PersistentCacheStats::operator=(PersistentCacheStats const& rhs)

=== modified file 'tests/core/persistent_string_cache/persistent_string_cache_test.cpp'
--- tests/core/persistent_string_cache/persistent_string_cache_test.cpp	2015-08-05 00:11:19 +0000
+++ tests/core/persistent_string_cache/persistent_string_cache_test.cpp	2015-12-10 06:07:16 +0000
@@ -363,6 +363,26 @@
             EXPECT_EQ(miss_run_time, s3.longest_miss_run_time());
 
             s2.cache_path();  // Moved-from instance must be usable.
+
+            // Move constructor from internal instance.
+            PersistentCacheStats s4(move(c->stats()));
+            EXPECT_EQ(test_db, s4.cache_path());
+            EXPECT_EQ(CacheDiscardPolicy::lru_ttl, s4.policy());
+            EXPECT_EQ(1, s4.size());
+            EXPECT_EQ(2, s4.size_in_bytes());
+            EXPECT_EQ(1024, s4.max_size_in_bytes());
+            EXPECT_EQ(4, s4.hits());
+            EXPECT_EQ(5, s4.misses());
+            EXPECT_EQ(1, s4.hits_since_last_miss());
+            EXPECT_EQ(0, s4.misses_since_last_hit());
+            EXPECT_EQ(3, s.longest_hit_run());
+            EXPECT_EQ(4, s.longest_miss_run());
+            EXPECT_EQ(last_hit_time, s4.most_recent_hit_time());
+            EXPECT_EQ(last_miss_time, s4.most_recent_miss_time());
+            EXPECT_EQ(hit_run_time, s4.longest_hit_run_time());
+            EXPECT_EQ(miss_run_time, s4.longest_miss_run_time());
+
+            s.cache_path();  // Moved-from instance must be usable.
         }
 
         // To get coverage for copying and moving from the internal instance,