search

pistacheio / pistache

A high-performance REST toolkit written in C++
https://pistacheio.github.io/pistache/
Apache License 2.0
3.12k stars 688 forks source link

fix: File descriptor leaks for eventfd and timerfd_create #1158

Closed tyler92 closed 8 months ago

tyler92 commented 11 months ago

There was no close call for the file descriptors created with eventfd and timerfd_create. It led to a file descriptors leak. I'm really not sure if the fix may have a bad impact, but at least there is no leak now.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (fe5639a) 77.92% compared to head (92973b3) 78.11%. Report is 1 commits behind head on master.

:exclamation: Current head 92973b3 differs from pull request most recent head e6f27c2. Consider uploading reports for the commit e6f27c2 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1158 +/- ## ========================================== + Coverage 77.92% 78.11% +0.19% ========================================== Files 53 53 Lines 6990 7074 +84 ========================================== + Hits 5447 5526 +79 - Misses 1543 1548 +5 ``` | [Files](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio) | Coverage Δ | | |---|---|---| | [include/pistache/http.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwLmg=) | `86.48% <ø> (ø)` | | | [include/pistache/http\_header.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwX2hlYWRlci5o) | `91.56% <100.00%> (-0.25%)` | :arrow_down: | | [include/pistache/http\_headers.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwX2hlYWRlcnMuaA==) | `78.57% <ø> (-0.92%)` | :arrow_down: | | [include/pistache/os.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9vcy5o) | `100.00% <ø> (ø)` | | | [src/common/mime.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9taW1lLmNj) | `88.88% <100.00%> (-1.17%)` | :arrow_down: | | [src/common/os.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9vcy5jYw==) | `61.45% <100.00%> (+1.33%)` | :arrow_up: | | [src/server/endpoint.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL3NlcnZlci9lbmRwb2ludC5jYw==) | `87.95% <100.00%> (+0.45%)` | :arrow_up: | | [src/common/http\_headers.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwX2hlYWRlcnMuY2M=) | `86.41% <96.55%> (+5.96%)` | :arrow_up: | | [src/common/http.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwLmNj) | `75.46% <83.33%> (+0.10%)` | :arrow_up: | | [src/common/http\_header.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwX2hlYWRlci5jYw==) | `91.74% <92.59%> (+0.07%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/pistacheio/pistache/pull/1158/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tyler92 commented 11 months ago

ABI check has failed due to the destructor. I would appreciate any help with it. Also, some clangs check failed, but it looks like the failures aren't related to the PR.

tyler92 commented 11 months ago

Probably related: https://github.com/pistacheio/pistache/issues/44

kiplingw commented 11 months ago

Hey @tyler92. Thanks for your PR. Two requests:

tyler92 commented 11 months ago

@kiplingw Done. I've implemented a simple check of the /proc/self/fd directory with a message about Valgrind as a fallback. This test passed with the fix and failed without it. The Valgrind report without the fix just for reference:

Valgrind report ``` ==3110541== ==3110541== FILE DESCRIPTORS: 16 open (3 std) at exit. ==3110541== Open file descriptor 33: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x19635F: Pistache::Tcp::Listener::runThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918ADF: testing::TestSuite::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4928F5D: testing::internal::UnitTestImpl::RunAllTests() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x49467AB: bool testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 32: ==3110541== at 0x4E6102B: timerfd_create (syscall-template.S:120) ==3110541== by 0x192B39: Pistache::Http::TransportImpl::registerPoller(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B1DF1: Pistache::Aio::AsyncImpl::addHandler(std::shared_ptr const&, bool) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1977B0: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918ADF: testing::TestSuite::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4928F5D: testing::internal::UnitTestImpl::RunAllTests() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 31: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x192B2A: Pistache::Http::TransportImpl::registerPoller(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B1DF1: Pistache::Aio::AsyncImpl::addHandler(std::shared_ptr const&, bool) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1977B0: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 27: ==3110541== at 0x4E6102B: timerfd_create (syscall-template.S:120) ==3110541== by 0x192B39: Pistache::Http::TransportImpl::registerPoller(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B1DF1: Pistache::Aio::AsyncImpl::addHandler(std::shared_ptr const&, bool) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1977B0: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918ADF: testing::TestSuite::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4928F5D: testing::internal::UnitTestImpl::RunAllTests() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 26: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x192B2A: Pistache::Http::TransportImpl::registerPoller(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B1DF1: Pistache::Aio::AsyncImpl::addHandler(std::shared_ptr const&, bool) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1977B0: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 22: ==3110541== at 0x4E6102B: timerfd_create (syscall-template.S:120) ==3110541== by 0x192B39: Pistache::Http::TransportImpl::registerPoller(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B1DF1: Pistache::Aio::AsyncImpl::addHandler(std::shared_ptr const&, bool) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1977B0: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918ADF: testing::TestSuite::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4928F5D: testing::internal::UnitTestImpl::RunAllTests() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 21: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x192B2A: Pistache::Http::TransportImpl::registerPoller(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B1DF1: Pistache::Aio::AsyncImpl::addHandler(std::shared_ptr const&, bool) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1977B0: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 17: ==3110541== at 0x4E6102B: timerfd_create (syscall-template.S:120) ==3110541== by 0x192B39: Pistache::Http::TransportImpl::registerPoller(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B1DF1: Pistache::Aio::AsyncImpl::addHandler(std::shared_ptr const&, bool) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1977B0: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918ADF: testing::TestSuite::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4928F5D: testing::internal::UnitTestImpl::RunAllTests() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 16: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x192B2A: Pistache::Http::TransportImpl::registerPoller(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B1DF1: Pistache::Aio::AsyncImpl::addHandler(std::shared_ptr const&, bool) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1977B0: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4918192: testing::TestInfo::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 12: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B246F: Pistache::Aio::AsyncImpl::AsyncImpl(Pistache::Aio::Reactor*, unsigned long, std::__cxx11::basic_string, std::allocator > const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B05BD: Pistache::Aio::AsyncContext::makeImpl(Pistache::Aio::Reactor*) const (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B0053: Pistache::Aio::Reactor::init(Pistache::Aio::ExecutionContext const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x19774B: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 10: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B246F: Pistache::Aio::AsyncImpl::AsyncImpl(Pistache::Aio::Reactor*, unsigned long, std::__cxx11::basic_string, std::allocator > const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B05BD: Pistache::Aio::AsyncContext::makeImpl(Pistache::Aio::Reactor*) const (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B0053: Pistache::Aio::Reactor::init(Pistache::Aio::ExecutionContext const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x19774B: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 8: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B246F: Pistache::Aio::AsyncImpl::AsyncImpl(Pistache::Aio::Reactor*, unsigned long, std::__cxx11::basic_string, std::allocator > const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B05BD: Pistache::Aio::AsyncContext::makeImpl(Pistache::Aio::Reactor*) const (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B0053: Pistache::Aio::Reactor::init(Pistache::Aio::ExecutionContext const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x19774B: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== Open file descriptor 6: ==3110541== at 0x4E5FE8B: eventfd (syscall-template.S:120) ==3110541== by 0x1809D7: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&)::{lambda()#1}::operator()() const [clone .constprop.0] (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x180C12: Pistache::NotifyFd::bind(Pistache::Polling::Epoll&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B246F: Pistache::Aio::AsyncImpl::AsyncImpl(Pistache::Aio::Reactor*, unsigned long, std::__cxx11::basic_string, std::allocator > const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B05BD: Pistache::Aio::AsyncContext::makeImpl(Pistache::Aio::Reactor*) const (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1B0053: Pistache::Aio::Reactor::init(Pistache::Aio::ExecutionContext const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x19774B: Pistache::Tcp::Listener::bind(Pistache::Address const&) (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x193D2B: Pistache::Http::Endpoint::serveThreaded() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x1334AC: http_server_test_http_server_is_not_leaked_Test::TestBody() (in /home/misha/github.com/pistacheio/pistache/build/tests/run_http_server_test) ==3110541== by 0x4945704: void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x493D99A: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== by 0x4917677: testing::Test::Run() (in /usr/local/lib/libgtest.so.1.13.0) ==3110541== ==3110541== ==3110541== HEAP SUMMARY: ==3110541== in use at exit: 0 bytes in 0 blocks ==3110541== total heap usage: 646 allocs, 646 frees, 184,453 bytes allocated ==3110541== ==3110541== All heap blocks were freed -- no leaks are possible ==3110541== ==3110541== For lists of detected and suppressed errors, rerun with: -s ==3110541== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) ```

I see the test http_server_test.multiple_client_with_requests_to_multithreaded_server is failed due to timeout, but I'm not sure if this is related to my changes

kiplingw commented 11 months ago

I was going to say the same thing. It seems fine, but it's an area of the code base I'm less familiar with. @dennisjenkins75 thoughts?

tyler92 commented 8 months ago

@kiplingw @dennisjenkins75 Hi. Do we have any chance to review/merge it?

kiplingw commented 8 months ago

I'm fine with it. But while we wait for @dennisjenkins75's feedback, I'd suggest bumping the patch version in version.txt because the one you've got now is already old in lieu of a recent PR.

tyler92 commented 8 months ago

This needs to be updated in lieu of the most recent merged PR.

Done

kiplingw commented 8 months ago

Hey @tyler92. Thank you for your PR. While we await @dennisjenkins75, I can probably anticipate at least some of his feedback: I think it would be prudent if you were able to add a unit test somehow to verify the leak and the fix.

You might have to be creative by monitoring /proc/self/fd. Note that trying to open anything there, there will be, of course, an additional FD created by trying to introspect. So you'll have to think about that.

tyler92 commented 8 months ago

Hey @tyler92. Thank you for your PR. While we await @dennisjenkins75, I can probably anticipate at least some of his feedback: I think it would be prudent if you were able to add a unit test somehow to verify the leak and the fix.

You might have to be creative by monitoring /proc/self/fd. Note that trying to open anything there, there will be, of course, an additional FD created by trying to introspect. So you'll have to think about that.

Oh, yeah, thanks. For some reason, I lost my unit test while rebasing my branch. Please find the unit test in the latest push. The test failed without the patch, and it's passed with it.

dennisjenkins75 commented 8 months ago

Suggestion:

  1. Add a unit test helper library (that is only linked into unit tests).
  2. In that library, add the function for "getting count of open file descriptors for 'self'."
    • In the future, if we ever port to a non-Linux OS, we'll abstract the function here.
  3. Add a unit test for that function alone.
    • Call it twice in a row, should get same result.
    • Call it once, then open a file, call it again, expect count to go up one, close the file, call again, expect count to go down one.
  4. Use the new library function in your intended unit test.
  5. Remove any code about "use valgrind".... Valgrind and other canonical leak detectors are a given for hunting down leaks. Adding that here makes it a bit too prescritive.
kiplingw commented 8 months ago

And just so you know @tyler92, we're not trying to give you the runaround. Your PR in principal makes sense. We just want to make sure we don't make new problems.

tyler92 commented 8 months ago

@kiplingw @dennisjenkins75 I've added new target tests/helpers with the function for getting a count of open file descriptors and tests. I'm not sure about the style for the copyright header, CMakeLists.txt, and meson.build, so please let me know if you don't like something (even minor things)

kiplingw commented 8 months ago

I can't see any immediate issues that stand out, but interested to hear what @dennisjenkins75 says.