Value corrruption with a view pointing to a value within an std::optional

Question

I am trying to create a setting where an instance of a C++ class optionally owns the data. For this I'm creating an std::optional<Type>, and a view to this optional (or something supplied from outside). Consider the following snippet of a minimum reproducible example of a problem I am having with the use of std::optional towards solving this:

#include <algorithm>
#include <iostream>
#include <optional>
#include <string_view>

class Wrap {
 public:
  Wrap(int value, size_t size) : size_(size) {
    value_ = new int[size];
    std::fill(value_, value_ + size_, value);
    std::cout << "new Wrap(" << (void*)value_ << ", " << size_ << ")" << std::endl;
  }

  ~Wrap() {
    std::cout << "delete Wrap(" << (void*)value_ << ", " << size_ << ")" << std::endl;
    delete[] value_;
    size_ = 0;
  }

  const int *data() const { return value_; }
  size_t size() const { return size_; }

 private:
  int *value_;
  size_t size_;
};

template <class T>
struct Pair {
  T first;
  T second;
};

Pair<Wrap> wrap_from(int first, int second, size_t size) {
  return {
      .first = Wrap(first, size),   //
      .second = Wrap(second, size)  //
  };
}

Pair<std::string_view> view_from(const Pair<Wrap> &wrap) {
  return {
      .first = std::string_view(                               //
          reinterpret_cast<const char *>(wrap.first.data()),   //
          wrap.first.size()                                    //
          ),                                                   //
      .second = std::string_view(                              //
          reinterpret_cast<const char *>(wrap.second.data()),  //
          wrap.second.size()),                                 //
                                                               //
  };
}

class Holder {
 public:
  Holder(int first, int second, size_t size)
      : wrap_(wrap_from(first, second, size)), view_(view_from(*wrap_)) {
    std::cout << "access first(" << (void*)view_.first.data() << ", " << view_.first.size() << ")" << std::endl;
    std::cout << "access second(" << (void*)view_.second.data() << ", " << view_.second.size() << ")" << std::endl;

    std::cout << "first: " << *(int*)view_.first.data() << std::endl;
    std::cout << "second: " << *(int*)view_.second.data() << std::endl;
  }

 private:
  using WrapPair = Pair<Wrap>;
  std::optional<WrapPair> wrap_;
  Pair<std::string_view> view_;
};

int main() {
  Holder holder(3, 4, 10);  
  return 0;
}

This code provides the following output (see https://godbolt.org/z/3r1MrYY7P):

Program returned: 139
Program stdout

new Wrap(0x563f294a62b0, 10)
new Wrap(0x563f294a72f0, 10)
delete Wrap(0x563f294a72f0, 10)
delete Wrap(0x563f294a62b0, 10)
access first(0x563f294a62b0, 10)
access second(0x563f294a72f0, 10)
first: 692744944
second: 0
delete Wrap(0x563f294a72f0, 10)

Program stderr

free(): double free detected in tcache 2
Program terminated with signal: SIGSEGV

I am trying to wrap my head around where the destructor is being called, rendering the values inside the std::optional useless. A double free also appears to be happening. I expect since most of these are in return types move-semantics and copy-elision kicks in.

What am I doing wrong? How to mitigate this situation? Are there alternate ways to achieve the intended effect?

Answer 1

Your Wrap class violates the rule of 3.

Adding the missing functions:

  Wrap(const Wrap& rhs) : value_(new int[rhs.size_])
  {
     std::copy(rhs.value_, rhs.value_ + rhs.size_, value_);
     size_ = rhs.size_;
  }
  
  Wrap& operator=(Wrap rhs)
  {
    std::swap(rhs.size_, size_);
    std::swap(rhs.value_, value_);
    return *this;
  }

The runtime errors are now alleviated.

Live example

A better illustration of the copying being done is to output the value of this and other information in the constructors and destructor:

Live example 2