[Core Services] Callbacks are now executed in separate threads #2458

FlorianReimold · 2026-01-08T10:30:05Z

Important service Callbacks are now executed in separate threads in order to not block the net-code. Threads are only created when needed, are stored in a dynamic thread pool and are re-used for latter callback executions.

This fixes service deadlock issues that would occur when the user blocked service callbacks with blocking code.

Callbacks that are executed from this dynamic threadpool are:

Server service callbacks (i.e. the user-provided code that actually defines what the service does)
Client service-call callbacks (i.e. the user provided code that gets executed upon service-call-completion or errors)

Event callbacks are still being executed synchronously in the netcode.

…y fixes the issue without breaking anything

…er code

…tfix/blocking_services

…on. Removed serialized service callbacks (->internal API change), as those were not used anyways

…tfix/blocking_services

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 31. Check the log or trigger a new build to see more.

ecal/core/src/service/ecal_service_server_impl.cpp

ecal/dynamic_threadpool/dynamic_threadpool/include/dynamic_threadpool/dynamic_threadpool.h

ecal/dynamic_threadpool/dynamic_threadpool/src/dynamic_threadpool.cpp

ecal/dynamic_threadpool/dynamic_threadpool/src/dynamic_threadpool_impl.cpp

ecal/dynamic_threadpool/tests/src/dynamic_threadpool_test.cpp

github-actions · 2026-01-08T10:49:29Z

ecal/dynamic_threadpool/tests/src/dynamic_threadpool_test.cpp

+
+  const std::chrono::milliseconds delay(500);
+
+  DynamicThreadPool thread_pool(max_threadpool_size);


warning: variable 'thread_pool' of type 'DynamicThreadPool' can be declared 'const' [misc-const-correctness]

Suggested change

DynamicThreadPool thread_pool(max_threadpool_size);

DynamicThreadPool const thread_pool(max_threadpool_size);

ecal/dynamic_threadpool/tests/src/dynamic_threadpool_test.cpp

…-ecal/ecal into hotfix/blocking_services

KerstinKeller

The threadpool is a great improvement 👍

Even though it's already merged, i do have a few remarks:

Please check the license headers, they contain the wrong year.
One recommendation using a std::optional for max_size
Some recommendation for the tests, to speed up execution time and check used thread ids instead of using heuristic to check which threads have been used.

I didn't review the actual usage in ecal_service.

KerstinKeller · 2026-01-09T08:03:09Z

ecal/core/src/service/ecal_service_singleton_manager.cpp

 /* ========================= eCAL LICENSE =================================
 *
 * Copyright (C) 2016 - 2025 Continental Corporation
+ * Copyright 2025 AUMOVIO and subsidiaries. All rights reserved.


Suggested change

* Copyright 2025 AUMOVIO and subsidiaries. All rights reserved.

* Copyright 2026 AUMOVIO and subsidiaries. All rights reserved.

😉

KerstinKeller · 2026-01-09T08:05:45Z

ecal/core/src/service/ecal_service_singleton_manager.cpp

      return nullptr;
    }

+    std::shared_ptr<DynamicThreadPool> ServiceManager::get_dynamic_threadpool()


Just for my understanding: Do I understand correctly, that the same DynamicThreadPool is used in both Service and Client implementations? (makes a lot of sense)

KerstinKeller · 2026-01-09T08:07:47Z

ecal/core/src/service/ecal_service_singleton_manager.cpp

+#include "dynamic_threadpool/dynamic_threadpool.h"
+#include "ecal_service/client_manager.h"
+#include "ecal_service/logger.h"
+#include "ecal_service/server_manager.h"


Why did you have to add the client_manager and server manager includes here? Also the logger? From the changes in this file it doesn't seem necessary.

KerstinKeller · 2026-01-09T08:08:01Z

ecal/core/src/service/ecal_service_singleton_manager.h

 /* ========================= eCAL LICENSE =================================
 *
 * Copyright (C) 2016 - 2025 Continental Corporation
+ * Copyright 2025 AUMOVIO and subsidiaries. All rights reserved.


Suggested change

* Copyright 2025 AUMOVIO and subsidiaries. All rights reserved.

* Copyright 2026 AUMOVIO and subsidiaries. All rights reserved.

KerstinKeller · 2026-01-09T08:08:31Z

ecal/core/src/service/ecal_service_server_impl.cpp

 /* ========================= eCAL LICENSE =================================
 *
 * Copyright (C) 2016 - 2025 Continental Corporation
 * Copyright 2025 AUMOVIO and subsidiaries. All rights reserved.


Suggested change

* Copyright 2025 - 2026 AUMOVIO and subsidiaries. All rights reserved.

KerstinKeller · 2026-01-09T08:44:44Z

ecal/dynamic_threadpool/tests/src/dynamic_threadpool_test.cpp

+
+    thread_pool.reset();
+  }
+


These two scopes are totally independent tests, so I would actually make them two independent testcases 😄

KerstinKeller · 2026-01-09T08:57:05Z

ecal/dynamic_threadpool/tests/src/dynamic_threadpool_test.cpp

+  }
+
+  // Wait a short time, so that the tasks have not finished yet
+  std::this_thread::sleep_for(delay / 2);


I wonder if there is a different way to check this without sleeping for fixed delays, which also increases total execution time.

I don't know if this is checked in, but for test purposes I implemented synchronization points. So you can synchronize all the threads and the main thread. There you know that everyone is still executing, the main thread can check everything and then you release the execution again.

This way, the test will still execute, but execution time can be way faster, which is only 500ms here, but can accumulate over time.

KerstinKeller · 2026-01-09T09:50:09Z

ecal/dynamic_threadpool/tests/src/dynamic_threadpool_test.cpp

+  EXPECT_LT(duration, delay * 2);
+
+  // Post again to verify that the threadpool reuses threads
+  finished_tasks = 0;


Maybe a better way to check reusage is to check thread id's (std::this_thread::get_id()). You could let the functions return their thread ID (hmhm or pass in a mutable reference) and later check the size of the set for example.

KerstinKeller · 2026-01-09T09:51:27Z

ecal/dynamic_threadpool/tests/src/dynamic_threadpool_test.cpp

+  EXPECT_EQ(thread_pool.GetSize(),      max_threadpool_size);
+  EXPECT_EQ(thread_pool.GetIdleCount(), max_threadpool_size);
+
+  // Check that the total time taken is "not much" more than two times the delay (We had twice amount of tasks than max threads)


same here, better use a std::set<std::thread::id> and check the size of the set.

KerstinKeller · 2026-01-09T09:52:10Z

ecal/dynamic_threadpool/tests/src/dynamic_threadpool_test.cpp

+
+  // Shutdown the threadpool while tasks are running
+  thread_pool.Shutdown();
+


Same, using a barrier might be more rubust and faster.

FlorianReimold added 13 commits November 25, 2025 16:08

Started implementing a test to test deeply nested services calls

15e13a5

Implemented tests for blocking server and client callbacks

7d1436b

Temporary quick-and-dirty fix to verify that spawning threads actuall…

82630c4

…y fixes the issue without breaking anything

Implemented a dynamic threadpool that can later be used to execute us…

f6982bf

…er code

fixed clang tidy warnings

5283eda

Merge branch 'master' of https://github.com/eclipse-ecal/ecal into ho…

df51da2

…tfix/blocking_services

Added stress-test test for dynamic_threadpool

9ac0439

eCAL Service Server now uses a proper dynamic threadpool implementati…

a38fcf3

…on. Removed serialized service callbacks (->internal API change), as those were not used anyways

Implemented Threadpool in client

257127d

Added a test for async callbacks

e95b6fc

Fixed service samples

e975de9

simplified calling of error callbacks

778604f

Merge branch 'master' of https://github.com/eclipse-ecal/ecal into ho…

4172f9d

…tfix/blocking_services

FlorianReimold added the cherry-pick-to-NONE Don't cherry-pick these changes label Jan 8, 2026

FlorianReimold linked an issue Jan 8, 2026 that may be closed by this pull request

Service Calls sometimes block indefnitively #2426

Closed

Removed obsolete todo comments

92e1634

github-actions bot reviewed Jan 8, 2026

View reviewed changes

FlorianReimold added 5 commits January 8, 2026 11:51

Added documentation

c8595f7

fixed clang tidy warnings

b22ae55

fixed include cleaner warnigns

03c7b6a

removed captured variables

dcdbaf5

Merge branch 'hotfix/blocking_services' of https://github.com/eclipse…

368057c

…-ecal/ecal into hotfix/blocking_services

hannemn approved these changes Jan 9, 2026

View reviewed changes

hannemn merged commit 9274436 into master Jan 9, 2026
37 checks passed

KerstinKeller reviewed Jan 9, 2026

View reviewed changes


		const std::chrono::milliseconds delay(500);

		DynamicThreadPool thread_pool(max_threadpool_size);

	DynamicThreadPool thread_pool(max_threadpool_size);
	DynamicThreadPool const thread_pool(max_threadpool_size);

	* Copyright 2025 AUMOVIO and subsidiaries. All rights reserved.
	* Copyright 2026 AUMOVIO and subsidiaries. All rights reserved.


	* Copyright 2025 - 2026 AUMOVIO and subsidiaries. All rights reserved.


		// Shutdown the threadpool while tasks are running
		thread_pool.Shutdown();

[Core Services] Callbacks are now executed in separate threads #2458

[Core Services] Callbacks are now executed in separate threads #2458

Uh oh!

Conversation

FlorianReimold commented Jan 8, 2026

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KerstinKeller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants