From 91c5954fe27eb020e2de422be53c5fe0c9c6cd6b Mon Sep 17 00:00:00 2001 From: ArthurSonzogni Date: Mon, 21 Jun 2021 22:49:42 +0200 Subject: [PATCH] Fix component ownership. When switching from raw pointers toward shared_ptr, the destructor wasn't updated correctly. This patch: - Fixes the issue. - Add two regression tests. - Use address sanitizer for the tests. This fixes: https://github.com/ArthurSonzogni/FTXUI/issues/115 --- CMakeLists.txt | 3 +++ src/ftxui/component/component.cpp | 4 ++- src/ftxui/component/component_test.cpp | 36 ++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 src/ftxui/component/component_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index fce83fc..1794a7e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -231,6 +231,7 @@ if (FTXUI_BUILD_TESTS AND ${CMAKE_VERSION} VERSION_GREATER "3.11.4") endif() add_executable(tests + src/ftxui/component/component_test.cpp src/ftxui/component/container_test.cpp src/ftxui/component/input_test.cpp src/ftxui/component/radiobox_test.cpp @@ -252,6 +253,8 @@ if (FTXUI_BUILD_TESTS AND ${CMAKE_VERSION} VERSION_GREATER "3.11.4") target_include_directories(tests PRIVATE src ) + target_compile_options(tests PRIVATE -fsanitize=address) + target_link_libraries(tests PRIVATE -fsanitize=address) set_property(TARGET tests PROPERTY CXX_STANDARD 17) diff --git a/src/ftxui/component/component.cpp b/src/ftxui/component/component.cpp index 1459ef7..9e9e7fd 100644 --- a/src/ftxui/component/component.cpp +++ b/src/ftxui/component/component.cpp @@ -19,7 +19,8 @@ class CaptureMouseImpl : public CapturedMouseInterface { } // namespace ComponentBase::~ComponentBase() { - Detach(); + while (children_.size() != 0) + children_.back()->Detach(); } /// @brief Return the parent ComponentBase, or nul if any. @@ -142,6 +143,7 @@ void ComponentBase::Detach() { return this == that.get(); }); parent_->children_.erase(it); + parent_ = nullptr; } } // namespace ftxui diff --git a/src/ftxui/component/component_test.cpp b/src/ftxui/component/component_test.cpp new file mode 100644 index 0000000..20792fc --- /dev/null +++ b/src/ftxui/component/component_test.cpp @@ -0,0 +1,36 @@ +#include // for Message +#include // for TestPartResult, SuiteApiResolver, TestFactoryImpl +#include // for __shared_ptr_access, shared_ptr, allocator + +#include "ftxui/component/component_base.hpp" +#include "gtest/gtest_pred_impl.h" // for AssertionResult, EXPECT_EQ, EXPECT_FALSE, EXPECT_TRUE, Test, TEST + +using namespace ftxui; + +namespace { +Component Make() { + return std::make_shared(); +} +} // namespace + +// Regression test for: +// https://github.com/ArthurSonzogni/FTXUI/issues/115 +TEST(ContainerTest, DeleteParentFirst) { + auto parent = Make(); + auto child = Make(); + parent->Add(child); + parent.reset(); + child.reset(); +} + +TEST(ContainerTest, DeleteChildFirst) { + auto parent = Make(); + auto child = Make(); + parent->Add(child); + child.reset(); + parent.reset(); +} + +// Copyright 2020 Arthur Sonzogni. All rights reserved. +// Use of this source code is governed by the MIT license that can be found in +// the LICENSE file.