fix shared_ptr self assignments

see open-telemetry#3713 (comment)

Author: Reneg973

api/include/opentelemetry/context/context.h

@@ -3,7 +3,7 @@
 
 #pragma once
 
-#include <string>
+#include <cstring>
 #include <utility>
 
 #include "opentelemetry/context/context_value.h"
@@ -34,18 +34,18 @@ class Context
 
   // Creates a context object from a key and value, this will
   // hold a shared_ptr to the head of the DataList linked list
-  Context(nostd::string_view key, const ContextValue &value) noexcept
+  Context(nostd::string_view key, ContextValue value) noexcept
       : head_{nostd::shared_ptr<DataList>{new DataList(key, value)}}
   {}
 
   // Accepts a new iterable and then returns a new context that
   // contains the new key and value data. It attaches the
   // exisiting list to the end of the new list.
   template <class T>
-  Context SetValues(T &values) const noexcept
+  Context SetValues(T &values) noexcept
   {
-    Context context(values);
-    auto last = context.head_;
+    Context context                  = Context(values);
+    nostd::shared_ptr<DataList> last = context.head_;
     while (last->next_ != nullptr)
     {
       last = last->next_;
@@ -57,9 +57,9 @@ class Context
   // Accepts a new iterable and then returns a new context that
   // contains the new key and value data. It attaches the
   // exisiting list to the end of the new list.
-  Context SetValue(nostd::string_view key, const ContextValue &value) const noexcept
+  Context SetValue(nostd::string_view key, ContextValue value) noexcept
   {
-    Context context(key, value);
+    Context context      = Context(key, value);
     context.head_->next_ = head_;
     return context;
   }
@@ -69,9 +69,12 @@ class Context
   {
     for (DataList *data = head_.get(); data != nullptr; data = data->next_.get())
     {
-      if (key == data->key_)
+      if (key.size() == data->key_length_)
       {
-        return data->value_;
+        if (std::memcmp(key.data(), data->key_, data->key_length_) == 0)
+        {
+          return data->value_;
+        }
       }
     }
     return ContextValue{};
@@ -89,10 +92,12 @@ class Context
   // A linked list to contain the keys and values of this context node
   struct DataList
   {
-    std::string key_;
+    char *key_ = nullptr;
 
     nostd::shared_ptr<DataList> next_{nullptr};
 
+    size_t key_length_ = 0UL;
+
     ContextValue value_;
 
     DataList() = default;
@@ -101,23 +106,62 @@ class Context
     template <class T>
     DataList(const T &keys_and_vals)
     {
-      auto iter = std::begin(keys_and_vals);
-      if (iter == std::end(keys_and_vals))
-        return;
+      bool first = true;
       auto *node = this;
-      *node      = DataList(iter->first, iter->second);
-      for (++iter; iter != std::end(keys_and_vals); ++iter)
+      for (auto &iter : keys_and_vals)
       {
-        node->next_ = nostd::shared_ptr<DataList>(new DataList(iter->first, iter->second));
-        node        = node->next_.get();
+        if (first)
+        {
+          *node = DataList(iter.first, iter.second);
+          first = false;
+        }
+        else
+        {
+          node->next_ = nostd::shared_ptr<DataList>(new DataList(iter.first, iter.second));
+          node        = node->next_.get();
+        }
       }
     }
 
     // Builds a data list with just a key and value, so it will just be the head
     // and returns that head.
     DataList(nostd::string_view key, const ContextValue &value)
-        : key_(key.begin(), key.end()), value_(value)
-    {}
+    {
+      key_        = new char[key.size()];
+      key_length_ = key.size();
+      std::memcpy(key_, key.data(), key.size() * sizeof(char));
+      next_  = nostd::shared_ptr<DataList>{nullptr};
+      value_ = value;
+    }
+
+    DataList(const DataList &other)
+        : key_(new char[other.key_length_]),
+          next_(other.next_),
+          key_length_(other.key_length_),
+          value_(other.value_)
+    {
+      std::memcpy(key_, other.key_, other.key_length_ * sizeof(char));
+    }
+
+    DataList &operator=(DataList &&other) noexcept
+    {
+      key_length_ = other.key_length_;
+      value_      = std::move(other.value_);
+      next_       = std::move(other.next_);
+
+      key_       = other.key_;
+      other.key_ = nullptr;
+
+      return *this;
+    }
+
+    ~DataList()
+    {
+      if (key_ != nullptr)
+      {
+        delete[] key_;
+      }
+    }
   };
 
   // Head of the list which holds the keys and values of this context

api/include/opentelemetry/nostd/shared_ptr.h

@@ -11,6 +11,7 @@
 #endif
 
 #if !defined(OPENTELEMETRY_HAVE_STD_SHARED_PTR)
+#  include <cstdlib>
 #  include <memory>
 #  include <utility>
 
@@ -31,9 +32,13 @@ class shared_ptr
   using pointer      = element_type *;
 
 private:
+  static constexpr size_t kMaxSize   = 32;
   static constexpr size_t kAlignment = 8;
 
-  struct alignas(kAlignment) PlacementBuffer;  // fwd.
+  struct alignas(kAlignment) PlacementBuffer
+  {
+    char data[kMaxSize]{};
+  };
 
   class shared_ptr_wrapper
   {
@@ -42,12 +47,14 @@ class shared_ptr
 
     shared_ptr_wrapper(std::shared_ptr<T> &&ptr) noexcept : ptr_{std::move(ptr)} {}
 
-    void CopyTo(PlacementBuffer &buffer) const noexcept
+    virtual ~shared_ptr_wrapper() {}
+
+    virtual void CopyTo(PlacementBuffer &buffer) const noexcept
     {
       new (buffer.data) shared_ptr_wrapper{*this};
     }
 
-    void MoveTo(PlacementBuffer &buffer) noexcept
+    virtual void MoveTo(PlacementBuffer &buffer) noexcept
     {
       new (buffer.data) shared_ptr_wrapper{std::move(this->ptr_)};
     }
@@ -60,19 +67,15 @@ class shared_ptr
       new (buffer.data) other_shared_ptr_wrapper{std::move(this->ptr_)};
     }
 
-    pointer Get() const noexcept { return ptr_.get(); }
+    virtual pointer Get() const noexcept { return ptr_.get(); }
 
-    void Reset() noexcept { ptr_.reset(); }
+    virtual void Reset() noexcept { ptr_.reset(); }
 
   private:
     std::shared_ptr<T> ptr_;
   };
 
-  struct alignas(kAlignment) PlacementBuffer
-  {
-    char data[sizeof(shared_ptr_wrapper)]{};
-  };
-
+  static_assert(sizeof(shared_ptr_wrapper) <= kMaxSize, "Placement buffer is too small");
   static_assert(alignof(shared_ptr_wrapper) <= kAlignment, "Placement buffer not properly aligned");
 
 public:
@@ -108,7 +111,7 @@ class shared_ptr
 
   shared_ptr(std::unique_ptr<T> &&other) noexcept
   {
-    std::shared_ptr<T> ptr_(std::move(other));
+    std::shared_ptr<T> ptr_(other.release());
     new (buffer_.data) shared_ptr_wrapper{std::move(ptr_)};
   }
 

api/test/nostd/shared_ptr_test.cc

@@ -99,7 +99,7 @@ TEST(SharedPtrTest, MoveConstructionFromStdSharedPtr)
 
 TEST(SharedPtrTest, MoveConstructionFromNoStdUniquePtr)
 {
-  opentelemetry::v1::nostd::unique_ptr<int> value(new int{123});
+  opentelemetry::nostd::unique_ptr<int> value(new int{123});
   auto p = value.get();
   shared_ptr<int> ptr{std::move(value)};
   EXPECT_EQ(value.get(), nullptr);  // NOLINT
@@ -133,6 +133,12 @@ TEST(SharedPtrTest, Assignment)
   std::shared_ptr<int> ptr3(value3);
   ptr1 = ptr3;
   EXPECT_EQ(ptr1.get(), value3);
+
+  ptr1 = ptr1;
+  EXPECT_EQ(ptr1.get(), ptr1.get());
+
+  ptr1 = std::move(ptr1);
+  EXPECT_EQ(ptr1.get(), ptr1.get());
 }
 
 TEST(SharedPtrTest, BoolConversionOpertor)