From 54992e72f140834d71b68e30b809b13e025d5050 Mon Sep 17 00:00:00 2001 From: Francisco Paisana Date: Thu, 28 Nov 2019 10:56:01 +0000 Subject: [PATCH] fixed and simplified multiqueue task api to avoid dangling pointers. --- lib/include/srslte/common/multiqueue.h | 39 ++++++++++++-------------- srsenb/hdr/stack/enb_stack_lte.h | 3 +- srsenb/src/stack/enb_stack_lte.cc | 6 ++-- srsue/hdr/stack/ue_stack_lte.h | 5 ++-- srsue/src/stack/ue_stack_lte.cc | 6 ++-- 5 files changed, 27 insertions(+), 32 deletions(-) diff --git a/lib/include/srslte/common/multiqueue.h b/lib/include/srslte/common/multiqueue.h index 560270efb..2dd6fd0e1 100644 --- a/lib/include/srslte/common/multiqueue.h +++ b/lib/include/srslte/common/multiqueue.h @@ -230,36 +230,33 @@ private: * Specialization for tasks with content that is move-only **********************************************************/ -template -class moveable_task_t +class move_task_t { public: - moveable_task_t() = default; + move_task_t() = default; template - moveable_task_t(Func&& f) : func(std::forward(f)) + move_task_t(Func&& f) : task_ptr(new derived_task(std::forward(f))) { } - template - moveable_task_t(Func&& f, Capture&& c) : capture(std::forward(c)) - { - std::function ftmp{std::forward(f)}; - func = [this, ftmp]() { ftmp(std::move(capture)); }; - } - void operator()() { func(); } + void operator()() { (*task_ptr)(); } private: - std::function func; - Capture capture; + struct base_task { + virtual void operator()() = 0; + }; + template + struct derived_task : public base_task { + derived_task(Func&& f_) : f(std::forward(f_)) {} + void operator()() final { f(); } + + private: + Func f; + }; + + std::unique_ptr task_ptr; }; -template -moveable_task_t bind_task(Func&& f, Capture&& c) -{ - return moveable_task_t{std::forward(f), std::forward(c)}; -} - -template -using multiqueue_task_handler = multiqueue_handler >; +using multiqueue_task_handler = multiqueue_handler; } // namespace srslte diff --git a/srsenb/hdr/stack/enb_stack_lte.h b/srsenb/hdr/stack/enb_stack_lte.h index 2959d168c..dba0f5e55 100644 --- a/srsenb/hdr/stack/enb_stack_lte.h +++ b/srsenb/hdr/stack/enb_stack_lte.h @@ -146,8 +146,7 @@ private: phy_interface_stack_lte* phy = nullptr; // state - bool started = false; - using task_t = srslte::moveable_task_t; + bool started = false; srslte::multiqueue_task_handler pending_tasks; int enb_queue_id = -1, sync_queue_id = -1, mme_queue_id = -1, gtpu_queue_id = -1, mac_queue_id = -1; }; diff --git a/srsenb/src/stack/enb_stack_lte.cc b/srsenb/src/stack/enb_stack_lte.cc index 2026cb2e2..95d5c7262 100644 --- a/srsenb/src/stack/enb_stack_lte.cc +++ b/srsenb/src/stack/enb_stack_lte.cc @@ -210,7 +210,7 @@ bool enb_stack_lte::get_metrics(stack_metrics_t* metrics) void enb_stack_lte::run_thread() { while (started) { - task_t task{}; + srslte::move_task_t task{}; if (pending_tasks.wait_pop(&task) >= 0) { task(); } @@ -223,11 +223,11 @@ void enb_stack_lte::handle_mme_rx_packet(srslte::unique_byte_buffer_t pdu, int flags) { // Defer the handling of MME packet to eNB stack main thread - auto task_handler = [this, from, sri, flags](srslte::unique_byte_buffer_t t) { + auto task_handler = [this, from, sri, flags](srslte::unique_byte_buffer_t& t) { s1ap.handle_mme_rx_msg(std::move(t), from, sri, flags); }; // Defer the handling of MME packet to main stack thread - pending_tasks.push(mme_queue_id, srslte::bind_task(task_handler, std::move(pdu))); + pending_tasks.push(mme_queue_id, std::bind(task_handler, std::move(pdu))); } void enb_stack_lte::add_mme_socket(int fd) diff --git a/srsue/hdr/stack/ue_stack_lte.h b/srsue/hdr/stack/ue_stack_lte.h index cbbb1566b..48c3d9feb 100644 --- a/srsue/hdr/stack/ue_stack_lte.h +++ b/srsue/hdr/stack/ue_stack_lte.h @@ -159,9 +159,8 @@ private: gw_interface_stack* gw = nullptr; // Thread - static const int STACK_MAIN_THREAD_PRIO = -1; // Use default high-priority below UHD - using task_t = srslte::moveable_task_t; - srslte::multiqueue_task_handler pending_tasks; + static const int STACK_MAIN_THREAD_PRIO = -1; // Use default high-priority below UHD + srslte::multiqueue_task_handler pending_tasks; int sync_queue_id = -1, ue_queue_id = -1, gw_queue_id = -1, mac_queue_id = -1, background_queue_id = -1; srslte::task_thread_pool background_tasks; ///< Thread pool used for long, low-priority tasks }; diff --git a/srsue/src/stack/ue_stack_lte.cc b/srsue/src/stack/ue_stack_lte.cc index 7018e398e..9f887f8ec 100644 --- a/srsue/src/stack/ue_stack_lte.cc +++ b/srsue/src/stack/ue_stack_lte.cc @@ -219,7 +219,7 @@ bool ue_stack_lte::get_metrics(stack_metrics_t* metrics) void ue_stack_lte::run_thread() { while (running) { - task_t task{}; + srslte::move_task_t task{}; if (pending_tasks.wait_pop(&task) >= 0) { task(); } @@ -242,10 +242,10 @@ void ue_stack_lte::run_thread() */ void ue_stack_lte::write_sdu(uint32_t lcid, srslte::unique_byte_buffer_t sdu, bool blocking) { - auto task = [this, lcid, blocking](srslte::unique_byte_buffer_t sdu) { + auto task = [this, lcid, blocking](srslte::unique_byte_buffer_t& sdu) { pdcp.write_sdu(lcid, std::move(sdu), blocking); }; - bool ret = pending_tasks.try_push(gw_queue_id, srslte::bind_task(task, std::move(sdu))).first; + bool ret = pending_tasks.try_push(gw_queue_id, std::bind(task, std::move(sdu))).first; if (not ret) { pdcp_log.warning("GW SDU with lcid=%d was discarded.\n", lcid); }