diff --git a/core/templates/cowdata.h b/core/templates/cowdata.h index dbbb1e6b540f..1ee36c6fa1eb 100644 --- a/core/templates/cowdata.h +++ b/core/templates/cowdata.h @@ -269,22 +269,28 @@ void CowData::_unref() { SafeNumeric *refc = _get_refcount(); if (refc->decrement() > 0) { - return; // still in use + // Data are still in use elsewhere. + _ptr = nullptr; + return; } - // clean up + // Clean up. + // First, invalidate our own reference. + // If we are involved in a cyclic reference, we may otherwise accidentally + // expose the buffer even though it's in the middle of being destructed. + // This is illegal behavior anyway, but we can mitigate the damage by + // invalidating our own reference before calling the destructors. + USize current_size = *_get_size(); + T *ptr = _ptr; + _ptr = nullptr; if constexpr (!std::is_trivially_destructible_v) { - USize current_size = *_get_size(); - for (USize i = 0; i < current_size; ++i) { - // call destructors - T *t = &_ptr[i]; - t->~T(); + ptr[i].~T(); } } // free mem - Memory::free_static(((uint8_t *)_ptr) - DATA_OFFSET, false); + Memory::free_static(((uint8_t *)ptr) - DATA_OFFSET, false); } template @@ -341,7 +347,6 @@ Error CowData::resize(Size p_size) { if (p_size == 0) { // wants to clean up _unref(); - _ptr = nullptr; return OK; } @@ -485,7 +490,6 @@ void CowData::_ref(const CowData &p_from) { } _unref(); - _ptr = nullptr; if (!p_from._ptr) { return; //nothing to do diff --git a/tests/core/templates/test_vector.h b/tests/core/templates/test_vector.h index 4152a8258cc4..a4616f38e7c6 100644 --- a/tests/core/templates/test_vector.h +++ b/tests/core/templates/test_vector.h @@ -532,6 +532,31 @@ TEST_CASE("[Vector] Operators") { CHECK(vector != vector_other); } +struct CyclicVectorHolder { + Vector *vector = nullptr; + bool is_destructing = false; + + ~CyclicVectorHolder() { + if (is_destructing) { + // The vector must exist and not expose its backing array at this point. + CHECK_NE(vector, nullptr); + CHECK_EQ(vector->ptr(), nullptr); + } + } +}; + +TEST_CASE("[Vector] Cyclic Reference") { + // Create a stack-space vector. + Vector vector; + // Add a new (empty) element. + vector.resize(1); + // Expose the vector to its element through CyclicVectorHolder. + // This is questionable behavior, but should still behave graciously. + vector.ptrw()[0] = CyclicVectorHolder { &vector }; + vector.ptrw()[0].is_destructing = true; + // The vector goes out of scope and destructs, calling CyclicVectorHolder's destructor. +} + } // namespace TestVector #endif // TEST_VECTOR_H