From ad46fc006fbd3dd0d7aa2a961521bad78ddd8b88 Mon Sep 17 00:00:00 2001 From: Xavier Arteaga Date: Wed, 18 Dec 2019 16:53:47 +0100 Subject: [PATCH] srsLTE: Fix thread memory leak. Moved test. Fix CLang warnings. --- lib/src/common/CMakeLists.txt | 2 + lib/src/common/test/CMakeLists.txt | 31 +++++++++++++ .../src/common/test/thread_pool_test.cc | 3 +- lib/src/common/test/thread_test.cc | 46 +++++++++++++++++++ lib/src/common/threads.c | 24 ++++++---- srsue/test/phy/CMakeLists.txt | 13 ++---- .../{phy_worker_test.cc => ue_phy_test.cc} | 0 7 files changed, 99 insertions(+), 20 deletions(-) create mode 100644 lib/src/common/test/CMakeLists.txt rename srsue/test/phy/phy_concurrency_test.cc => lib/src/common/test/thread_pool_test.cc (98%) create mode 100644 lib/src/common/test/thread_test.cc rename srsue/test/phy/{phy_worker_test.cc => ue_phy_test.cc} (100%) diff --git a/lib/src/common/CMakeLists.txt b/lib/src/common/CMakeLists.txt index adf0485e9..61deac81f 100644 --- a/lib/src/common/CMakeLists.txt +++ b/lib/src/common/CMakeLists.txt @@ -58,3 +58,5 @@ add_executable(arch_select arch_select.cc) target_include_directories(srslte_common PUBLIC ${SEC_INCLUDE_DIRS}) target_link_libraries(srslte_common srslte_phy ${SEC_LIBRARIES}) install(TARGETS srslte_common DESTINATION ${LIBRARY_DIR}) + +add_subdirectory(test) \ No newline at end of file diff --git a/lib/src/common/test/CMakeLists.txt b/lib/src/common/test/CMakeLists.txt new file mode 100644 index 000000000..612d9af87 --- /dev/null +++ b/lib/src/common/test/CMakeLists.txt @@ -0,0 +1,31 @@ +# +# Copyright 2013-2019 Software Radio Systems Limited +# +# This file is part of srsLTE +# +# srsLTE is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of +# the License, or (at your option) any later version. +# +# srsLTE is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# A copy of the GNU Affero General Public License can be found in +# the LICENSE file in the top-level directory of this distribution +# and at http://www.gnu.org/licenses/. +# + +add_executable(thread_pool_test thread_pool_test.cc) +target_link_libraries(thread_pool_test + srslte_common) +add_test(thread_pool_test thread_pool_test) + +add_executable(thread_test thread_test.cc) +target_link_libraries(thread_test + srsue_phy + srslte_common) +add_test(thread_test thread_test) + diff --git a/srsue/test/phy/phy_concurrency_test.cc b/lib/src/common/test/thread_pool_test.cc similarity index 98% rename from srsue/test/phy/phy_concurrency_test.cc rename to lib/src/common/test/thread_pool_test.cc index 8a9bdaaf8..ace73d1a6 100644 --- a/srsue/test/phy/phy_concurrency_test.cc +++ b/lib/src/common/test/thread_pool_test.cc @@ -18,11 +18,10 @@ * and at http://www.gnu.org/licenses/. * */ +#include #include #include #include -#include -#include class dummy_radio { diff --git a/lib/src/common/test/thread_test.cc b/lib/src/common/test/thread_test.cc new file mode 100644 index 000000000..357ffe510 --- /dev/null +++ b/lib/src/common/test/thread_test.cc @@ -0,0 +1,46 @@ +/* + * Copyright 2013-2019 Software Radio Systems Limited + * + * This file is part of srsLTE. + * + * srsLTE is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of + * the License, or (at your option) any later version. + * + * srsLTE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * A copy of the GNU Affero General Public License can be found in + * the LICENSE file in the top-level directory of this distribution + * and at http://www.gnu.org/licenses/. + * + */ + +#include +#include + +class thread_test : public thread +{ +public: + thread_test() : thread("Threat Test") {} + +protected: + void run_thread() override { std::cout << "Hello world!" << std::endl; } +}; + +int main(int argc, char** argv) +{ + // Declare thread + thread_test mythread; + + // Start + mythread.start(0); + + // Wait to finish, join + mythread.wait_thread_finish(); + + return 0; +} \ No newline at end of file diff --git a/lib/src/common/threads.c b/lib/src/common/threads.c index 437153c8b..0a53e7715 100644 --- a/lib/src/common/threads.c +++ b/lib/src/common/threads.c @@ -90,7 +90,11 @@ bool threads_new_rt_cpu(pthread_t* thread, void* (*start_routine)(void*), void* // All threads have normal priority except prio_offset=0,1,2,3,4 if (prio_offset >= 0 && prio_offset < 5) { param.sched_priority = 50 - prio_offset; - pthread_attr_init(&attr); + if (pthread_attr_init(&attr)) { + perror("pthread_attr_init"); + } else { + attr_enable = true; + } if (pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED)) { perror("pthread_attr_setinheritsched"); } @@ -101,12 +105,14 @@ bool threads_new_rt_cpu(pthread_t* thread, void* (*start_routine)(void*), void* perror("pthread_attr_setschedparam"); fprintf(stderr, "Error not enough privileges to set Scheduling priority\n"); } - attr_enable = true; - } else { #endif param.sched_priority = 0; - pthread_attr_init(&attr); + if (pthread_attr_init(&attr)) { + perror("pthread_attr_init"); + } else { + attr_enable = true; + } if (pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED)) { perror("pthread_attr_setinheritsched"); } @@ -117,16 +123,15 @@ bool threads_new_rt_cpu(pthread_t* thread, void* (*start_routine)(void*), void* perror("pthread_attr_setschedparam"); fprintf(stderr, "Error not enough privileges to set Scheduling priority\n"); } - attr_enable = true; } if (cpu > 0) { if (cpu > 50) { - int mask; + uint32_t mask; mask = cpu / 100; CPU_ZERO(&cpuset); - for (int i = 0; i < 8; i++) { - if (((mask >> i) & 0x01) == 1) { + for (uint32_t i = 0; i < 8; i++) { + if (((mask >> i) & 0x01U) == 1U) { CPU_SET((size_t)i, &cpuset); } } @@ -143,6 +148,9 @@ bool threads_new_rt_cpu(pthread_t* thread, void* (*start_routine)(void*), void* int err = pthread_create(thread, attr_enable ? &attr : NULL, start_routine, arg); if (err) { if (EPERM == err) { + // Join failed thread for avoiding memory leak from previous trial + pthread_join(*thread, NULL); + perror("Warning: Failed to create thread with real-time priority. Creating it with normal priority"); err = pthread_create(thread, NULL, start_routine, arg); if (err) { diff --git a/srsue/test/phy/CMakeLists.txt b/srsue/test/phy/CMakeLists.txt index 6c438e3e4..4f1697a65 100644 --- a/srsue/test/phy/CMakeLists.txt +++ b/srsue/test/phy/CMakeLists.txt @@ -29,8 +29,8 @@ link_directories( ${SEC_LIBRARY_DIRS} ) -add_executable(phy_worker_test phy_worker_test.cc) -target_link_libraries(phy_worker_test +add_executable(ue_phy_test ue_phy_test.cc) +target_link_libraries(ue_phy_test srsue_phy srsue_stack srsue_upper @@ -43,14 +43,7 @@ target_link_libraries(phy_worker_test rrc_asn1 ${CMAKE_THREAD_LIBS_INIT} ${Boost_LIBRARIES}) -add_test(phy_worker_test phy_worker_test) - -add_executable(phy_concurrency_test phy_concurrency_test.cc) -target_link_libraries(phy_concurrency_test - srsue_phy - srslte_common) -add_test(phy_concurrency_test phy_concurrency_test) - +add_test(ue_phy_test ue_phy_test) add_executable(scell_search_test scell_search_test.cc) target_link_libraries(scell_search_test diff --git a/srsue/test/phy/phy_worker_test.cc b/srsue/test/phy/ue_phy_test.cc similarity index 100% rename from srsue/test/phy/phy_worker_test.cc rename to srsue/test/phy/ue_phy_test.cc