0

I have the following code that ends up calling the destructor twice on an object and crashing. What is the best practice way of implementing this design to avoid such issues?

class DataStore {

private:
    uint8_t* data;
    int dataLength;

public:
    DataStore(int length){
        data = new uint8_t[length];
        dataLength = length;
    }

    ~DataStore(){
        printf("DataStore Destructor\n");
        delete [] data;
    }
};

class DataStoreHelper {
private:
    DataStore _dataStore;
public:
    DataStoreHelper(DataStore& dataStore) : _dataStore(dataStore){

    }
};

int main(int argc, char** argv) {
    DataStore dataStore(10);
    DataStoreHelper dataStoreHelper(dataStore);
}

With the following output:

DataStore Destructor
DataStore Destructor
free(): double free detected in tcache 2

I could fix the problem by storing _dataStore as a pointer, but I really want to know is this the best practice method of implementation or is there a better way?

Peter Smith
  • 1
  • The default generated copy constructor and assignment operator functions do a shallow copy of your internally managed array. You have to implement these yourself dojng a deep copy of `data`. – πάντα ῥεῖ Mar 17 '21 at 00:42
  • @πάνταῥεῖ: More precisely, they copy the pointer itself, not the array, so both point to the same array, and both try to `delete` it independently. – ShadowRanger Mar 17 '21 at 00:43
  • @ShadowRanger _"More precisely, they copy the pointer itself"_ When I was young that was called a _shallow copy_, exactly what I mentioned. I didn't know that terminology changed about that the last 30 years, sorry. – πάντα ῥεῖ Mar 17 '21 at 00:46
  • @πάνταῥεῖ: Nothing you wrote was wrong, nothing to be sorry about. It's not that the terminology changed, it's just it's ambiguous in some contexts. Some languages have a stricter differentiation (e.g. most languages where all objects are on the heap, like Python, where `lst2 = lst1` aliases/pointer copies, `lst2 = lst1[:]` shallow copies, and `lst2 = copy.deepcopy(lst1)` does a fully recursive copy to arbitrary depth). For C++ programmers, it's generally fully shallow or fully deep (when written correctly), but since the OP is clearly new to C++, I figured I'd be extra specific. – ShadowRanger Mar 17 '21 at 00:53
  • Thanks for your responses, I see now that I don't want a copy of the object and a pointer is what I should be using here. And in the case I do want a copy, I should be doing a deep copy for allocated memory. – Peter Smith Mar 17 '21 at 00:55
  • 2
    @PeterSmith I suppose you're doing that as an excercise. If not, you don't need to write that code yourself, but can simply use [`std::vector`](https://en.cppreference.com/w/cpp/container/vector) – πάντα ῥεῖ Mar 17 '21 at 01:02

0 Answers0