ArthurSonzogni / FTXUI

:computer: C++ Functional Terminal User Interface. :heart:
MIT License
6.73k stars 401 forks source link

[Valgrind] Invalid write of size 8 #302

Closed vnepogodin closed 2 years ago

vnepogodin commented 2 years ago

Problem:

Getting that with valgrind. All components affected.

==118808== Invalid write of size 8
==118808==    at 0x12FA5E: ftxui::ComponentBase::Detach() (component.cpp:68)
==118808==    by 0x12FAB8: ftxui::ComponentBase::DetachAllChildren() (component.cpp:75)
==118808==    by 0x12F85F: ftxui::ComponentBase::~ComponentBase() (component.cpp:22)
==118808==    by 0x149B0B: ftxui::(anonymous namespace)::ResizableSplitLeftBase::~ResizableSplitLeftBase() (resizable_split.cpp:15)
==118808==    by 0x14A358: void __gnu_cxx::new_allocator<ftxui::(anonymous namespace)::ResizableSplitLeftBase>::destroy<ftxui::(anonymous namespace)::ResizableSplitLeftBase>(ftxui::(anonymous namespace)::ResizableSplitLeftBase*) (new_allocator.h:168)
==118808==    by 0x14A288: void std::allocator_traits<std::allocator<ftxui::(anonymous namespace)::ResizableSplitLeftBase> >::destroy<ftxui::(anonymous namespace)::ResizableSplitLeftBase>(std::allocator<ftxui::(anonymous namespace)::ResizableSplitLeftBase>&, ftxui::(anonymous namespace)::ResizableSplitLeftBase*) (alloc_traits.h:535)
==118808==    by 0x14A094: std::_Sp_counted_ptr_inplace<ftxui::(anonymous namespace)::ResizableSplitLeftBase, std::allocator<ftxui::(anonymous namespace)::ResizableSplitLeftBase>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:528)
==118808==    by 0x127F34: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:168)
==118808==    by 0x125D4A: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:705)
==118808==    by 0x124AEB: std::__shared_ptr<ftxui::ComponentBase, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1154)
==118808==    by 0x124B07: std::shared_ptr<ftxui::ComponentBase>::~shared_ptr() (shared_ptr.h:122)
==118808==    by 0x11FACD: main (homescreen.cpp:473)

Expected: too see zero output related to that error from valgrind.

How to reproduce:

git clone https://github.com/ArthurSonzogni/FTXUI.git
cd FTXUI
cmake -S . -B build -GNinja -DCMAKE_BUILD_TYPE=Debug
cmake --build build
valgrind --num-callers=20 --leak-check=yes --leak-resolution=high --track-origins=yes --show-reachable=yes ./build/examples/component/ftxui_example_homescreen
ArthurSonzogni commented 2 years ago

I guess: https://github.com/ArthurSonzogni/FTXUI/blob/84299de2e11522ad1d531a50f7610678ae0e5e67/src/ftxui/component/component.cpp#L59-L69

The parent detach its child. The child updates its parent pointer to nullptr. The problem happens when the detaching cause the ref count to go to zero and delete the child. One solution would be to update |parent_| to nullptr earlier.

Probably something like:

void ComponentBase::Detach() {
  if (!parent_)
    return;
  auto it = std::find_if(std::begin(parent_->children_),  //
                         std::end(parent_->children_),    //
                         [this](const Component& that) {  //
                           return this == that.get();
                         });
  ComponentBase* parent = parent_;
  parent_ = nullptr;
  parent->children_.erase(it);  // Might delete |this|.
}