search

google / cppdap

C++ library for the Debug Adapter Protocol
Apache License 2.0
152 stars 46 forks source link

possibly memory leak reported by valgrind #122

Open libinwa opened 11 months ago

libinwa commented 11 months ago

Hi all, I created a demo application based on the 1.58.0-a version of this DAP implementation. After doing the memory check by the valgrind tool, I get a possibly lost report as following:

--------------------------------------------------------
  672 bytes in 2 blocks are possibly lost in loss record 578 of 581
--------------------------------------------------------
    at 0x484DA83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
     by 0x40147D9: calloc (rtld-malloc.h:44)
     by 0x40147D9: allocate_dtv (dl-tls.c:375)
     by 0x40147D9: _dl_allocate_tls (dl-tls.c:634)
     by 0x819E7B4: allocate_stack (allocatestack.c:430)
     by 0x819E7B4: pthread_create@@GLIBC_2.34 (pthread_create.c:647)
     by 0x7EB2328: std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (no file available)
     by 0x7CB3AA9: std::thread::thread<(anonymous namespace)::Impl::startProcessingMessages(std::function<void ()> const&)::{lambda()#1}, , void>((anonymous namespace)::Impl::startProcessingMessages(std::function<void ()> const&)::{lambda()#1}&&) (std_thread.h:143)
     by 0x7CAF1AD: (anonymous namespace)::Impl::startProcessingMessages(std::function<void ()> const&) (session.cpp:86)
     by 0x4BAEEE: dap::Session::bind(std::shared_ptr<dap::Reader> const&, std::shared_ptr<dap::Writer> const&, std::function<void ()> const&) (session.h:450)
     by 0x4BAF57: dap::Session::bind(std::shared_ptr<dap::ReaderWriter> const&, std::function<void ()> const&) (session.h:455)

When I investigated the call stack, I found it was caused by a termination of the dispatchingThread and recvThread without join procedure.

Finally, I found the root cause is destructor method at "session.cpp:~Impl()" missing the virtual keyword. After modifying ~Impl() to virtual ~Impl(), the issue disappeared.

ben-clayton commented 11 months ago

Thank you for your bug report.

I'm confused by this. The base class (Session) already has a virtual destructor:

https://github.com/google/cppdap/blob/main/include/dap/session.h#L142

Because of this, I don't think there's any semantic difference between having a virtual on the derived class' destructor or not. Likewise override has no semantic meaning (that I'm aware of), but the other virtual methods all have it, so I've created a PR to mark the destructor with override in https://github.com/google/cppdap/pull/123