From 05fd1241f1ac1565c120e30b129bc2fcc094f58a Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Thu, 30 Jan 2025 10:56:19 +0100 Subject: [PATCH 1/9] Refs #22624: Regression test Signed-off-by: Juanjo Garcia --- test/unittest/logging/LogTests.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/unittest/logging/LogTests.cpp b/test/unittest/logging/LogTests.cpp index dd9e9fbab62..f1a1652cafe 100644 --- a/test/unittest/logging/LogTests.cpp +++ b/test/unittest/logging/LogTests.cpp @@ -695,6 +695,30 @@ TEST_F(LogTests, thread_config) EXPECT_EQ(entries.size(), n_logs); } +/** + * Regression test 22624: when setting thread affinity fails, eprosima log error throws another error, + * and calls eprosima log error. This causes a looping recursive call for eprosima log error. + */ +TEST_F(LogTests, thread_log_error_loop) +{ + // Set general verbosity + Log::SetVerbosity(Log::Error); + + // Set thread settings + eprosima::fastdds::rtps::ThreadSettings thr_settings{}; +#ifdef __linux__ + thr_settings.affinity = 0xFFFFFFFFFFFFFFFF; +#elif _WIN32 + thr_settings.affinity = 0xFFFFFFFFFFFFFFFF; +#elif __APPLE__ + thr_settings.affinity = 0xFFFFFFFFFFFFFFFF; +#endif + Log::SetThreadConfig(thr_settings); + + // Start the error message + EPROSIMA_LOG_ERROR(SYSTEM, "Did I start the recursive loop?"); +} + int main( int argc, char** argv) From df75fd66a74e4d7edf335216076ec9aab7c28656 Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Thu, 30 Jan 2025 12:48:33 +0100 Subject: [PATCH 2/9] Refs #22624: Corrected bug Signed-off-by: Juanjo Garcia --- src/cpp/utils/threading/threading_osx.ipp | 20 +++++++++++++---- src/cpp/utils/threading/threading_pthread.ipp | 22 ++++++++++++++----- src/cpp/utils/threading/threading_win32.ipp | 15 +++++++++++-- test/unittest/logging/LogTests.cpp | 4 ++-- 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/cpp/utils/threading/threading_osx.ipp b/src/cpp/utils/threading/threading_osx.ipp index 5fa21ee6dde..fd0dedf81e6 100644 --- a/src/cpp/utils/threading/threading_osx.ipp +++ b/src/cpp/utils/threading/threading_osx.ipp @@ -23,6 +23,18 @@ #include #include +#define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ + do{ \ + if (strcmp(thread_name, "dds.log") == 0) \ + { \ + std::cerr << msg << std::endl; \ + } \ + else \ + { \ + EPROSIMA_LOG_ERROR(SYSTEM, msg); \ + } \ + } while(0) + namespace eprosima { template @@ -98,13 +110,13 @@ static void configure_current_thread_scheduler( result = setpriority(PRIO_PROCESS, tid, sched_priority); if (0 != result) { - EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set priority of thread with id [" << tid << "," << thread_name << "] to value " << sched_priority << ". Error '" << strerror( + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set priority of thread with id [" << tid << "," << thread_name << "] to value " << sched_priority << ". Error '" << strerror( result) << "'"); } } else if (0 != result) { - EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << ". Error '" << strerror( + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << ". Error '" << strerror( result) << "'"); } } @@ -118,7 +130,7 @@ static void configure_current_thread_scheduler( result = pthread_setschedparam(self_tid, sched_class, ¶m); if (0 != result) { - EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << " with priority " << param.sched_priority << ". Error '" << strerror( + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << " with priority " << param.sched_priority << ". Error '" << strerror( result) << "'"); } } @@ -138,7 +150,7 @@ static void configure_current_thread_affinity( 1); if (0 != result) { - EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set affinity of thread with id [" << self_tid << "," << thread_name << "] to value " << affinity << ". Error '" << strerror( + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set affinity of thread with id [" << self_tid << "," << thread_name << "] to value " << affinity << ". Error '" << strerror( result) << "'"); } } diff --git a/src/cpp/utils/threading/threading_pthread.ipp b/src/cpp/utils/threading/threading_pthread.ipp index e09f891a240..412c6691089 100644 --- a/src/cpp/utils/threading/threading_pthread.ipp +++ b/src/cpp/utils/threading/threading_pthread.ipp @@ -34,6 +34,18 @@ #define gettid() ((pid_t)syscall(SYS_gettid)) #endif +#define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ + do{ \ + if (strcmp(thread_name, "dds.log") == 0) \ + { \ + std::cerr << msg << std::endl; \ + } \ + else \ + { \ + EPROSIMA_LOG_ERROR(SYSTEM, msg); \ + } \ + } while(0) + namespace eprosima { template @@ -114,13 +126,13 @@ static void configure_current_thread_scheduler( result = setpriority(PRIO_PROCESS, gettid(), sched_priority); if (0 != result) { - EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set priority of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_priority << ". Error '" << strerror( + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set priority of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_priority << ". Error '" << strerror( result) << "'"); } } else if (0 != result) { - EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << ". Error '" << strerror( + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << ". Error '" << strerror( result) << "'"); } } @@ -135,7 +147,7 @@ static void configure_current_thread_scheduler( result = pthread_setschedparam(self_tid, sched_class, ¶m); if (0 != result) { - EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << " with priority " << param.sched_priority << ". Error '" << strerror( + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << " with priority " << param.sched_priority << ". Error '" << strerror( result) << "'"); } } @@ -178,7 +190,7 @@ static void configure_current_thread_affinity( if (affinity_mask > 0) { - EPROSIMA_LOG_ERROR(SYSTEM, "Affinity mask has more processors than the ones present in the system"); + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Affinity mask has more processors than the ones present in the system"); } if (result > 0) @@ -192,7 +204,7 @@ static void configure_current_thread_affinity( if (0 != result) { - EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set affinity of thread with id [" << self_tid << "," << thread_name << "] to value " << affinity_mask << ". Error '" << strerror( + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set affinity of thread with id [" << self_tid << "," << thread_name << "] to value " << affinity_mask << ". Error '" << strerror( result) << "'"); } } diff --git a/src/cpp/utils/threading/threading_win32.ipp b/src/cpp/utils/threading/threading_win32.ipp index 60ea4acf7ab..dd0e34151f5 100644 --- a/src/cpp/utils/threading/threading_win32.ipp +++ b/src/cpp/utils/threading/threading_win32.ipp @@ -20,6 +20,17 @@ #include +#define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ + do{ \ + if (strcmp(thread_name, "dds.log") == 0) \ + { \ + std::cerr << msg << std::endl; \ + } \ + else \ + { \ + EPROSIMA_LOG_ERROR(SYSTEM, msg); \ + } \ + } while(0) namespace eprosima { template @@ -69,7 +80,7 @@ static void configure_current_thread_priority( { if (0 == SetThreadPriority(GetCurrentThread(), priority)) { - EPROSIMA_LOG_ERROR(SYSTEM, + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set priority of thread with id [" << GetCurrentThreadId() << "," << thread_name << "] to value " << priority << ". Error '" << GetLastError() << "'"); } @@ -84,7 +95,7 @@ static void configure_current_thread_affinity( { if (0 == SetThreadAffinityMask(GetCurrentThread(), static_cast(affinity_mask))) { - EPROSIMA_LOG_ERROR(SYSTEM, + THREAD_EPROSIMA_LOG_ERROR(thread_name, "Problem to set affinity of thread with id [" << GetCurrentThreadId() << "," << thread_name << "] to value " << affinity_mask << ". Error '" << GetLastError() << "'"); } diff --git a/test/unittest/logging/LogTests.cpp b/test/unittest/logging/LogTests.cpp index f1a1652cafe..1a25a1d9691 100644 --- a/test/unittest/logging/LogTests.cpp +++ b/test/unittest/logging/LogTests.cpp @@ -712,11 +712,11 @@ TEST_F(LogTests, thread_log_error_loop) thr_settings.affinity = 0xFFFFFFFFFFFFFFFF; #elif __APPLE__ thr_settings.affinity = 0xFFFFFFFFFFFFFFFF; -#endif +#endif // ifdef __linux__ Log::SetThreadConfig(thr_settings); // Start the error message - EPROSIMA_LOG_ERROR(SYSTEM, "Did I start the recursive loop?"); + EPROSIMA_LOG_ERROR(SYSTEM, "Recursive error loop avoided"); } int main( From 82c0da8afa508de75ac2d0d82ee4ccd892242c07 Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Fri, 31 Jan 2025 09:06:13 +0100 Subject: [PATCH 3/9] Refs #22624: improved structure of the test Signed-off-by: Juanjo Garcia --- test/unittest/logging/LogTests.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/unittest/logging/LogTests.cpp b/test/unittest/logging/LogTests.cpp index 1a25a1d9691..18d78d07f40 100644 --- a/test/unittest/logging/LogTests.cpp +++ b/test/unittest/logging/LogTests.cpp @@ -706,13 +706,7 @@ TEST_F(LogTests, thread_log_error_loop) // Set thread settings eprosima::fastdds::rtps::ThreadSettings thr_settings{}; -#ifdef __linux__ thr_settings.affinity = 0xFFFFFFFFFFFFFFFF; -#elif _WIN32 - thr_settings.affinity = 0xFFFFFFFFFFFFFFFF; -#elif __APPLE__ - thr_settings.affinity = 0xFFFFFFFFFFFFFFFF; -#endif // ifdef __linux__ Log::SetThreadConfig(thr_settings); // Start the error message From e82b75e2cbdbbc90f3e76976273d77695c61a30c Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Wed, 5 Feb 2025 10:47:09 +0100 Subject: [PATCH 4/9] Refs #22624: added reviewer suggestions Signed-off-by: Juanjo Garcia --- src/cpp/utils/threading/threading.hpp | 29 +++++++++++++++++++ src/cpp/utils/threading/threading_osx.ipp | 13 +-------- src/cpp/utils/threading/threading_pthread.ipp | 13 +-------- src/cpp/utils/threading/threading_win32.ipp | 12 +------- 4 files changed, 32 insertions(+), 35 deletions(-) create mode 100644 src/cpp/utils/threading/threading.hpp diff --git a/src/cpp/utils/threading/threading.hpp b/src/cpp/utils/threading/threading.hpp new file mode 100644 index 00000000000..73166ff6ffb --- /dev/null +++ b/src/cpp/utils/threading/threading.hpp @@ -0,0 +1,29 @@ +// Copyright 2025 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/*! + * @file threading.hpp + */ + +#define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ + do{ \ + if (strcmp(thread_name, "dds.log") == 0) \ + { \ + std::cerr << msg << std::endl; \ + } \ + else \ + { \ + EPROSIMA_LOG_ERROR(SYSTEM, msg); \ + } \ + } while(0) \ No newline at end of file diff --git a/src/cpp/utils/threading/threading_osx.ipp b/src/cpp/utils/threading/threading_osx.ipp index fd0dedf81e6..bbac151f8a1 100644 --- a/src/cpp/utils/threading/threading_osx.ipp +++ b/src/cpp/utils/threading/threading_osx.ipp @@ -22,18 +22,7 @@ #include #include - -#define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ - do{ \ - if (strcmp(thread_name, "dds.log") == 0) \ - { \ - std::cerr << msg << std::endl; \ - } \ - else \ - { \ - EPROSIMA_LOG_ERROR(SYSTEM, msg); \ - } \ - } while(0) +#include namespace eprosima { diff --git a/src/cpp/utils/threading/threading_pthread.ipp b/src/cpp/utils/threading/threading_pthread.ipp index 412c6691089..2fde2fec2de 100644 --- a/src/cpp/utils/threading/threading_pthread.ipp +++ b/src/cpp/utils/threading/threading_pthread.ipp @@ -25,6 +25,7 @@ #include #include +#include #if defined(__GLIBC__) && ((__GLIBC__ > 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ <= 30))) #include @@ -34,18 +35,6 @@ #define gettid() ((pid_t)syscall(SYS_gettid)) #endif -#define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ - do{ \ - if (strcmp(thread_name, "dds.log") == 0) \ - { \ - std::cerr << msg << std::endl; \ - } \ - else \ - { \ - EPROSIMA_LOG_ERROR(SYSTEM, msg); \ - } \ - } while(0) - namespace eprosima { template diff --git a/src/cpp/utils/threading/threading_win32.ipp b/src/cpp/utils/threading/threading_win32.ipp index dd0e34151f5..478431e4bf5 100644 --- a/src/cpp/utils/threading/threading_win32.ipp +++ b/src/cpp/utils/threading/threading_win32.ipp @@ -19,18 +19,8 @@ #include #include +#include -#define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ - do{ \ - if (strcmp(thread_name, "dds.log") == 0) \ - { \ - std::cerr << msg << std::endl; \ - } \ - else \ - { \ - EPROSIMA_LOG_ERROR(SYSTEM, msg); \ - } \ - } while(0) namespace eprosima { template From 6753bfc0f57480039a68bf9f117a884fe49a0a26 Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Wed, 5 Feb 2025 10:47:35 +0100 Subject: [PATCH 5/9] Refs #22624: fixed reference to new file Signed-off-by: Juanjo Garcia --- src/cpp/utils/threading/threading.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/utils/threading/threading.hpp b/src/cpp/utils/threading/threading.hpp index 73166ff6ffb..03520fba070 100644 --- a/src/cpp/utils/threading/threading.hpp +++ b/src/cpp/utils/threading/threading.hpp @@ -26,4 +26,4 @@ { \ EPROSIMA_LOG_ERROR(SYSTEM, msg); \ } \ - } while(0) \ No newline at end of file + } while(0) From 02461e033af8102080267109012d1fdeabb011eb Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Wed, 5 Feb 2025 10:49:51 +0100 Subject: [PATCH 6/9] Refs #22624: fixed typo Signed-off-by: Juanjo Garcia --- src/cpp/utils/threading/threading.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/utils/threading/threading.hpp b/src/cpp/utils/threading/threading.hpp index 03520fba070..a0dad6c64ed 100644 --- a/src/cpp/utils/threading/threading.hpp +++ b/src/cpp/utils/threading/threading.hpp @@ -26,4 +26,4 @@ { \ EPROSIMA_LOG_ERROR(SYSTEM, msg); \ } \ - } while(0) + } while (0) From a5cdb0814d316dbdab2b0b566551aa6fa6ff0361 Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Wed, 5 Feb 2025 11:39:18 +0100 Subject: [PATCH 7/9] Refs #22624: Added reviwer suggestions Signed-off-by: Juanjo Garcia --- .../utils/threading/{threading.hpp => thread_logging.hpp} | 7 ++++++- src/cpp/utils/threading/threading_osx.ipp | 2 +- src/cpp/utils/threading/threading_pthread.ipp | 2 +- src/cpp/utils/threading/threading_win32.ipp | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) rename src/cpp/utils/threading/{threading.hpp => thread_logging.hpp} (93%) diff --git a/src/cpp/utils/threading/threading.hpp b/src/cpp/utils/threading/thread_logging.hpp similarity index 93% rename from src/cpp/utils/threading/threading.hpp rename to src/cpp/utils/threading/thread_logging.hpp index a0dad6c64ed..de99f9f8803 100644 --- a/src/cpp/utils/threading/threading.hpp +++ b/src/cpp/utils/threading/thread_logging.hpp @@ -13,9 +13,12 @@ // limitations under the License. /*! - * @file threading.hpp + * @file thread_logging.hpp */ +#ifndef THREADING_HPP +#define THREADING_HPP + #define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ do{ \ if (strcmp(thread_name, "dds.log") == 0) \ @@ -27,3 +30,5 @@ EPROSIMA_LOG_ERROR(SYSTEM, msg); \ } \ } while (0) + +#endif // THREADING_HPP diff --git a/src/cpp/utils/threading/threading_osx.ipp b/src/cpp/utils/threading/threading_osx.ipp index bbac151f8a1..aaf8adf930d 100644 --- a/src/cpp/utils/threading/threading_osx.ipp +++ b/src/cpp/utils/threading/threading_osx.ipp @@ -22,7 +22,7 @@ #include #include -#include +#include namespace eprosima { diff --git a/src/cpp/utils/threading/threading_pthread.ipp b/src/cpp/utils/threading/threading_pthread.ipp index 2fde2fec2de..de7b2624d1b 100644 --- a/src/cpp/utils/threading/threading_pthread.ipp +++ b/src/cpp/utils/threading/threading_pthread.ipp @@ -25,7 +25,7 @@ #include #include -#include +#include #if defined(__GLIBC__) && ((__GLIBC__ > 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ <= 30))) #include diff --git a/src/cpp/utils/threading/threading_win32.ipp b/src/cpp/utils/threading/threading_win32.ipp index 478431e4bf5..d476f5c01ad 100644 --- a/src/cpp/utils/threading/threading_win32.ipp +++ b/src/cpp/utils/threading/threading_win32.ipp @@ -19,7 +19,7 @@ #include #include -#include +#include namespace eprosima { From 873cc2d93cc0d50f4e07622d786652184016bb24 Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Wed, 5 Feb 2025 11:47:22 +0100 Subject: [PATCH 8/9] Refs #22624: modified guard Signed-off-by: Juanjo Garcia --- src/cpp/utils/threading/thread_logging.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/utils/threading/thread_logging.hpp b/src/cpp/utils/threading/thread_logging.hpp index de99f9f8803..0ea0a5a9d1e 100644 --- a/src/cpp/utils/threading/thread_logging.hpp +++ b/src/cpp/utils/threading/thread_logging.hpp @@ -16,8 +16,8 @@ * @file thread_logging.hpp */ -#ifndef THREADING_HPP -#define THREADING_HPP +#ifndef THREAD_LOGGING_HPP +#define THREAD_LOGGING_HPP #define THREAD_EPROSIMA_LOG_ERROR(thread_name, msg) \ do{ \ From 729662f453c96f24e5e43dfdddaff1ebfd40a3a0 Mon Sep 17 00:00:00 2001 From: Juanjo Garcia Date: Wed, 5 Feb 2025 12:53:29 +0100 Subject: [PATCH 9/9] Refs #22624: fixed typo Signed-off-by: Juanjo Garcia --- src/cpp/utils/threading/thread_logging.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/utils/threading/thread_logging.hpp b/src/cpp/utils/threading/thread_logging.hpp index 0ea0a5a9d1e..0fc191441f5 100644 --- a/src/cpp/utils/threading/thread_logging.hpp +++ b/src/cpp/utils/threading/thread_logging.hpp @@ -31,4 +31,4 @@ } \ } while (0) -#endif // THREADING_HPP +#endif // THREAD_LOGGING_HPP