MB-30097: Use valueSize() instead of using size directly

The higher order bit of the size of a Blob will be set if the value if
uncompressible. The valueSize() function will clear the bit and return
the actual size of the value. The valueSize() function needs to be used
in the copy contructor of the Blob to use a memcpy instead of using
the size variable directly.

Change-Id: I7745190e83c1ee97c4fdefc3ac06473627159549
Reviewed-on: http://review.couchbase.org/95566
Well-Formed: Build Bot <[email protected]>
Tested-by: Build Bot <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>

Author: Sriram Ganesan

engines/ep/src/blob.cc

@@ -55,10 +55,10 @@ Blob::Blob(const size_t len) : Blob(nullptr, len) {
 }
 
 Blob::Blob(const Blob& other)
-    : size(other.size),
+    : size(other.size.load()),
       // While this is a copy, it is a new allocation therefore reset age.
       age(0) {
-    std::memcpy(data, other.data, size);
+    std::memcpy(data, other.data, other.valueSize());
     ObjectRegistry::onCreateBlob(this);
 }
 

engines/ep/src/blob.h

@@ -161,8 +161,10 @@ class Blob : public RCValue {
 
     // Size of the value. The highest bit is used to represent if the
     // value is compressible or not. If set, then the value is not
-    // compressible.
-    uint32_t size;
+    // compressible. This needs to be an atomic variable as there could
+    // be a data race between threads that update the size
+    // (e.g, the setUncompressible API) and the ones that read the size
+    std::atomic<uint32_t> size;
 
     // The age of this Blob, in terms of some unspecified units of time.
     uint8_t age;

engines/ep/src/item.cc

@@ -205,7 +205,7 @@ std::ostream& operator<<(std::ostream& os, const Blob& b) {
        << " age:" << int(b.age)
        << " data: <" << std::hex;
     // Print at most 40 bytes of the body.
-    auto bytes_to_print = std::min(uint32_t(40), b.size);
+    auto bytes_to_print = std::min(uint32_t(40), b.size.load());
     for (size_t ii = 0; ii < bytes_to_print; ii++) {
         if (ii != 0) {
             os << ' ';

engines/ep/tests/module_tests/stored_value_test.cc

@@ -135,6 +135,24 @@ TYPED_TEST(ValueTest, DISABLED_StoredValueReallocateGivesSameSize) {
     }
 }
 
+/* MB-30097: Set the stored value as uncompressible and ensure that
+ * the size of the Blob doesn't change on a reallocation
+ */
+TYPED_TEST(ValueTest, StoredValueUncompressibleReallocateGivesSameSize) {
+    auto sv = this->factory(
+            make_item(0,
+                      makeStoredDocKey(std::string(10, 'k').c_str()),
+                      std::string(182, 'v').c_str()),
+            {});
+
+    auto blob = sv->getValue();
+    int beforeValueSize = blob->valueSize();
+    sv->setUncompressible();
+    sv->reallocate();
+    blob = sv->getValue();
+    EXPECT_EQ(beforeValueSize, blob->valueSize());
+}
+
 TYPED_TEST(ValueTest, metaDataSize) {
     // Check metadata size reports correctly.
     EXPECT_EQ(this->getFixedSize() + /*key*/ 3 + /*len*/ 1 +