From 72704fc26aca5b831d569a0a6bf46c0369f13426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois-Simon=20Fauteux-Chapleau?= <francois-simon.fauteux-chapleau@savoirfairelinux.com> Date: Wed, 15 May 2024 16:00:20 -0400 Subject: [PATCH] turn_transport: avoid leaking file descriptors This patch fixes two issues in the shutdown function that were causing file descriptors to remain open after they were no longer needed: 1) The TURN socket (`relay`) wasn't being destroyed properly 2) Several of the resources allocated in the TurnTransport constructor (such as the ioqueue and the timer_heap) weren't being cleaned up at all The first issue was introduced in a recent commit (2569341fdf1d896e0e8). The second one seems to have been present for a long time (at least over a year). Change-Id: I82e3f96b2a08737ce0a6e3ed5aaa53959de9b1e8 --- src/turn/turn_transport.cpp | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/turn/turn_transport.cpp b/src/turn/turn_transport.cpp index 49fdf59..0e36f46 100644 --- a/src/turn/turn_transport.cpp +++ b/src/turn/turn_transport.cpp @@ -84,17 +84,37 @@ public: // otherwise there's a potential race condition where pj_turn_sock_destroy // sets the state of the TURN session to PJ_TURN_STATE_DESTROYING, and then // ioWorker tries to execute a callback which expects the session to be in - // an earlier state. (This causes a crash if PJSIP was compiled with asserts - // enabled, see https://git.jami.net/savoirfairelinux/jami-daemon/-/issues/974) + // an earlier state. See https://git.jami.net/savoirfairelinux/dhtnet/-/issues/27 if (ioWorker.joinable()) { stopped_ = true; ioWorker.join(); } if (relay) { pj_turn_sock_destroy(relay); + // Calling pj_turn_sock_destroy doesn't (necessarily) immediately close the + // socket; as mentioned in PJSIP's documentation, the operation may be performed + // asynchronously, which is why we need to call the two polling functions below. + // https://docs.pjsip.org/en/latest/api/generated/pjnath/group/group__PJNATH__TURN__SOCK.html + const pj_time_val delay = {0, 20}; + pj_ioqueue_poll(stunConfig.ioqueue, &delay); + pj_timer_heap_poll(stunConfig.timer_heap, nullptr); relay = nullptr; } turnLock.reset(); + if (stunConfig.timer_heap) { + pj_timer_heap_destroy(stunConfig.timer_heap); + stunConfig.timer_heap = nullptr; + } + if (stunConfig.ioqueue) { + pj_ioqueue_destroy(stunConfig.ioqueue); + stunConfig.ioqueue = nullptr; + } + if (pool) { + pj_pool_release(pool); + pool = nullptr; + } + pj_pool_factory_dump(&poolCache.factory, PJ_TRUE); + pj_caching_pool_destroy(&poolCache); } TurnTransportParams settings; @@ -119,7 +139,6 @@ public: TurnTransport::Impl::~Impl() { shutdown(); - pj_caching_pool_destroy(&poolCache); } void -- GitLab