diff --git a/include/ice_options.h b/include/ice_options.h index c26b83eefd5dfba83604a2887ce7b378d8475db6..e7aee6d1cd5fa478f53fee6018bb0a7d7a55be05 100644 --- a/include/ice_options.h +++ b/include/ice_options.h @@ -8,6 +8,10 @@ namespace jami { +namespace upnp { +class UPnPContext; +} + class IceTransportFactory; using IceTransportCompleteCb = std::function<void(bool)>; @@ -61,6 +65,7 @@ struct IceTransportOptions // Addresses used by the account owning the transport instance. IpAddr accountLocalAddr {}; IpAddr accountPublicAddr {}; + std::shared_ptr<upnp::UPnPContext> upnpContext {}; }; } diff --git a/include/upnp/upnp_context.h b/include/upnp/upnp_context.h index a85db51082b381dd62d27f23f4b361de1bab7f5c..b9a5d266e8615497480b4a1b4b689adf839cabf7 100644 --- a/include/upnp/upnp_context.h +++ b/include/upnp/upnp_context.h @@ -37,6 +37,7 @@ #include "upnp/mapping.h" #include <opendht/rng.h> +#include <opendht/logger.h> #include <asio/steady_timer.hpp> #include <set> @@ -110,11 +111,11 @@ private: }; public: - UPnPContext(); + UPnPContext(std::shared_ptr<asio::io_context> ctx, std::shared_ptr<dht::log::Logger> logger); ~UPnPContext(); // Retrieve the UPnPContext singleton. - static std::shared_ptr<UPnPContext> getUPnPContext(); + // static std::shared_ptr<UPnPContext> getUPnPContext(); // Terminate the instance. void shutdown(); diff --git a/include/upnp/upnp_control.h b/include/upnp/upnp_control.h index 10e44e3e2e5d6a5598f22cee0d500801e25a45a1..e68cada394950dcebb6462308439f98e4b679862 100644 --- a/include/upnp/upnp_control.h +++ b/include/upnp/upnp_control.h @@ -40,7 +40,7 @@ class UPnPContext; class Controller { public: - Controller(); + Controller(const std::shared_ptr<UPnPContext>& ctx); ~Controller(); // Set known public address diff --git a/include/upnp/upnp_thread_util.h b/include/upnp/upnp_thread_util.h index 6c7f97b935f4fb4f26f56940cf034e97e596b966..6bc5271339a0cdff9061cf154f5f6f36b3b6a9b8 100644 --- a/include/upnp/upnp_thread_util.h +++ b/include/upnp/upnp_thread_util.h @@ -3,12 +3,13 @@ #include <thread> #include <memory> #include <asio/io_context.hpp> +#include <fmt/format.h> // This macro is used to validate that a code is executed from the expected // thread. It's useful to detect unexpected race on data members. #define CHECK_VALID_THREAD() \ if (not isValidThread()) \ - printf("The calling thread %d is not the expected thread: %d", getCurrentThread(), threadId_); + fmt::print("The calling thread {} is not the expected thread: {}\n", getCurrentThread(), threadId_); /*JAMI_ERR() << "The calling thread " << getCurrentThread() \ << " is not the expected thread: " << threadId_;*/ diff --git a/src/ice_transport.cpp b/src/ice_transport.cpp index 1b8ef839e66ef727e2f37a12bf4855f7289ab4e5..ff0b51f64216e4e1c1bd95b618bc25b54ff1d54f 100644 --- a/src/ice_transport.cpp +++ b/src/ice_transport.cpp @@ -422,7 +422,7 @@ IceTransport::Impl::initIceInstance(const IceTransportOptions& options) initiatorSession_ ? "master" : "slave"); if (upnpEnabled_) - upnp_.reset(new upnp::Controller()); + upnp_.reset(new upnp::Controller(options.upnpContext)); config_ = options.factory->getIceCfg(); // config copy if (isTcp_) { diff --git a/src/upnp/protocol/upnp_protocol.h b/src/upnp/protocol/upnp_protocol.h index 8ce08208389588037cea16106b9c5826964aa7d0..048f385b866a096b5c36021676af5ab83c4a5155 100644 --- a/src/upnp/protocol/upnp_protocol.h +++ b/src/upnp/protocol/upnp_protocol.h @@ -22,7 +22,7 @@ #pragma once #include "igd.h" -#include "mapping.h" +#include "upnp/mapping.h" #include "ip_utils.h" #include <map> diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp index 11112b7e671abdfd4c9d92566f22c61bf660312b..a36e56f9788133fdf44748d70619a1ddf807975e 100644 --- a/src/upnp/upnp_context.cpp +++ b/src/upnp/upnp_context.cpp @@ -21,7 +21,10 @@ */ #include "upnp/upnp_context.h" +#include "protocol/upnp_protocol.h" + #include <asio/steady_timer.hpp> +#include <fmt/std.h> namespace jami { namespace upnp { @@ -35,7 +38,8 @@ constexpr static uint16_t UPNP_TCP_PORT_MAX {UPNP_TCP_PORT_MIN + 5000}; constexpr static uint16_t UPNP_UDP_PORT_MIN {20000}; constexpr static uint16_t UPNP_UDP_PORT_MAX {UPNP_UDP_PORT_MIN + 5000}; -UPnPContext::UPnPContext() +UPnPContext::UPnPContext(std::shared_ptr<asio::io_context> ctx, std::shared_ptr<dht::log::Logger> logger) + : mappingListUpdateTimer_(*ioContext) { // JAMI_DBG("Creating UPnPContext instance [%p]", this); @@ -43,19 +47,16 @@ UPnPContext::UPnPContext() portRange_.emplace(PortType::TCP, std::make_pair(UPNP_TCP_PORT_MIN, UPNP_TCP_PORT_MAX)); portRange_.emplace(PortType::UDP, std::make_pair(UPNP_UDP_PORT_MIN, UPNP_UDP_PORT_MAX)); - if (not isValidThread()) { - runOnUpnpContextQueue([this] { init(); }); - return; - } + ioContext->post([this] { init(); }); } -std::shared_ptr<UPnPContext> +/*std::shared_ptr<UPnPContext> UPnPContext::getUPnPContext() { // This is the unique shared instance (singleton) of UPnPContext class. static auto context = std::make_shared<UPnPContext>(); return context; -} +}*/ void UPnPContext::shutdown(std::condition_variable& cv) @@ -71,8 +72,9 @@ UPnPContext::shutdown(std::condition_variable& cv) { std::lock_guard<std::mutex> lock(mappingMutex_); mappingList_->clear(); - if (mappingListUpdateTimer_) - mappingListUpdateTimer_->cancel(); + //if (mappingListUpdateTimer_) + // mappingListUpdateTimer_->cancel(); + mappingListUpdateTimer_.cancel(); controllerList_.clear(); protocolList_.clear(); shutdownComplete_ = true; @@ -310,7 +312,7 @@ UPnPContext::reserveMapping(Mapping& requestedMap) if (desiredPort == 0) { // JAMI_DBG("Desired port is not set, will provide the first available port for [%s]", - requestedMap.getTypeStr()); + // requestedMap.getTypeStr()); } else { // JAMI_DBG("Try to find mapping for port %i [%s]", desiredPort, requestedMap.getTypeStr()); } @@ -613,10 +615,11 @@ UPnPContext::updateMappingList(bool async) // Update the preferred IGD. updatePreferredIgd(); - if (mappingListUpdateTimer_) { + /*if (mappingListUpdateTimer_) { mappingListUpdateTimer_->cancel(); mappingListUpdateTimer_ = {}; - } + }*/ + mappingListUpdateTimer_.cancel(); // Skip if no controller registered. if (controllerList_.empty()) @@ -630,8 +633,13 @@ UPnPContext::updateMappingList(bool async) return; } - mappingListUpdateTimer_ = getScheduler()->scheduleIn([this] { updateMappingList(false); }, - MAP_UPDATE_INTERVAL); + /*mappingListUpdateTimer_ = getScheduler()->scheduleIn([this] { updateMappingList(false); }, + MAP_UPDATE_INTERVAL);*/ + mappingListUpdateTimer_.expires_from_now(MAP_UPDATE_INTERVAL); + mappingListUpdateTimer_.async_wait([this](asio::error_code const& ec) { + if (ec != asio::error::operation_aborted) + updateMappingList(false); + }); // Process pending requests if any. processPendingRequests(prefIgd); diff --git a/src/upnp/upnp_control.cpp b/src/upnp/upnp_control.cpp index 30de6da7e196e3f080f2eb7ccc4853d39cc841cd..bba37d31bfff9dfa3b90078c6affdcaa0fd3ba53 100644 --- a/src/upnp/upnp_control.cpp +++ b/src/upnp/upnp_control.cpp @@ -26,17 +26,10 @@ namespace jami { namespace upnp { -Controller::Controller() +Controller::Controller(const std::shared_ptr<UPnPContext>& ctx) + : upnpContext_(ctx) { - try { - upnpContext_ = UPnPContext::getUPnPContext(); - } catch (std::runtime_error& e) { - // JAMI_ERR("UPnP context error: %s", e.what()); - } - - assert(upnpContext_); upnpContext_->registerController(this); - // JAMI_DBG("Controller@%p: Created UPnP Controller session", this); }