From 2bec342d91a4568bf10d71a2e012249327b23181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=82osz=20Kosobucki?= Date: Thu, 27 Jun 2024 17:51:49 +0200 Subject: [PATCH] Implement auto invocation of deferred slots Connection invocations are now evaluated automatically in the event loop. Any time an invocation is enqueued using the event loop's connection evaluator, the event loop is woken up. This required a KDUtils-specific ConnectionEvaluator. When given event loop is destroyed, it sets a reference in the evaluator to null. It's a defensive measure for the future to prevent misuse of lingering evaluators in case the user deletes the event loop first. AbstractEventLoop::waitForEvents is not pure abstract anymore as it ensures evaluation of slot invocations. Removed one copypasted comment in android event loop impl. --- src/KDFoundation/CMakeLists.txt | 1 + .../platform/abstract_platform_event_loop.cpp | 70 +++++++++++++++++++ .../platform/abstract_platform_event_loop.h | 8 ++- .../linux/linux_platform_event_loop.cpp | 2 +- .../linux/linux_platform_event_loop.h | 4 +- .../macos/macos_platform_event_loop.h | 2 +- .../macos/macos_platform_event_loop.mm | 2 +- .../win32/win32_platform_event_loop.cpp | 2 +- .../win32/win32_platform_event_loop.h | 3 +- .../android/android_platform_event_loop.cpp | 2 +- .../android/android_platform_event_loop.h | 7 +- .../linux_wayland_platform_event_loop.cpp | 4 +- .../linux_wayland_platform_event_loop.h | 2 +- .../xcb/linux_xcb_platform_event_loop.cpp | 4 +- .../linux/xcb/linux_xcb_platform_event_loop.h | 2 +- tests/auto/utils/signal/tst_signal.cpp | 34 ++++++++- 16 files changed, 123 insertions(+), 26 deletions(-) create mode 100644 src/KDFoundation/platform/abstract_platform_event_loop.cpp diff --git a/src/KDFoundation/CMakeLists.txt b/src/KDFoundation/CMakeLists.txt index 240c868..9772ccb 100644 --- a/src/KDFoundation/CMakeLists.txt +++ b/src/KDFoundation/CMakeLists.txt @@ -18,6 +18,7 @@ set(SOURCES object.cpp postman.cpp timer.cpp + platform/abstract_platform_event_loop.cpp ) set(HEADERS diff --git a/src/KDFoundation/platform/abstract_platform_event_loop.cpp b/src/KDFoundation/platform/abstract_platform_event_loop.cpp new file mode 100644 index 0000000..2a4d69f --- /dev/null +++ b/src/KDFoundation/platform/abstract_platform_event_loop.cpp @@ -0,0 +1,70 @@ +/* + This file is part of KDUtils. + + SPDX-FileCopyrightText: 2024 Klarälvdalens Datakonsult AB, a KDAB Group company + Author: Miłosz Kosobucki + + SPDX-License-Identifier: MIT + + Contact KDAB at for commercial licensing options. +*/ + +#include +#include "abstract_platform_event_loop.h" + +namespace KDFoundation { + +/** + * KDUtils-specific connection evaluator that wakes up the owning event loop when slot invocation is + * enqueued via the loop's connection evaluator. + */ +class ConnectionEvaluator final : public KDBindings::ConnectionEvaluator +{ +public: + ConnectionEvaluator(AbstractPlatformEventLoop *eventLoop) + : m_eventLoop(eventLoop) { } + + void setEventLoop(AbstractPlatformEventLoop *eventLoop) + { + this->m_eventLoop = eventLoop; + } + +protected: + void onInvocationAdded() override + { + if (m_eventLoop) { + m_eventLoop->wakeUp(); + } + } + +private: + AbstractPlatformEventLoop *m_eventLoop = nullptr; +}; +} // namespace KDFoundation + +using namespace KDFoundation; + +KDFoundation::AbstractPlatformEventLoop::AbstractPlatformEventLoop() + : m_connectionEvaluator(new ConnectionEvaluator(this)) +{ +} + +AbstractPlatformEventLoop::~AbstractPlatformEventLoop() +{ + // Make sure that any remaining references to our connection evaluator aren't referring to a dead + // event loop object. + if (m_connectionEvaluator) { + std::static_pointer_cast(m_connectionEvaluator)->setEventLoop(nullptr); + } +} + +void KDFoundation::AbstractPlatformEventLoop::waitForEvents(int timeout) +{ + waitForEventsImpl(timeout); + + // Possibly we woke up because of deferred slot invocation was posted. Let's (possibly) + // execute them while we're at it. + if (m_connectionEvaluator) { + m_connectionEvaluator->evaluateDeferredConnections(); + } +} diff --git a/src/KDFoundation/platform/abstract_platform_event_loop.h b/src/KDFoundation/platform/abstract_platform_event_loop.h index eb8205d..323e87a 100644 --- a/src/KDFoundation/platform/abstract_platform_event_loop.h +++ b/src/KDFoundation/platform/abstract_platform_event_loop.h @@ -28,7 +28,8 @@ class Timer; class KDFOUNDATION_API AbstractPlatformEventLoop { public: - virtual ~AbstractPlatformEventLoop() { } + AbstractPlatformEventLoop(); + virtual ~AbstractPlatformEventLoop(); void setPostman(Postman *postman) { m_postman = postman; } Postman *postman() { return m_postman; } @@ -37,7 +38,7 @@ class KDFOUNDATION_API AbstractPlatformEventLoop // -1 means wait forever // 0 means do not wait (i.e. poll) // +ve number, wait for up to timeout msecs - virtual void waitForEvents(int timeout) = 0; + void waitForEvents(int timeout); // Kick the event loop out of waiting virtual void wakeUp() = 0; @@ -56,9 +57,10 @@ class KDFOUNDATION_API AbstractPlatformEventLoop protected: virtual std::unique_ptr createPlatformTimerImpl(Timer *timer) = 0; + virtual void waitForEventsImpl(int timeout) = 0; Postman *m_postman{ nullptr }; - std::shared_ptr m_connectionEvaluator{ new KDBindings::ConnectionEvaluator }; + std::shared_ptr m_connectionEvaluator; }; } // namespace KDFoundation diff --git a/src/KDFoundation/platform/linux/linux_platform_event_loop.cpp b/src/KDFoundation/platform/linux/linux_platform_event_loop.cpp index fc7c419..f3de4a6 100644 --- a/src/KDFoundation/platform/linux/linux_platform_event_loop.cpp +++ b/src/KDFoundation/platform/linux/linux_platform_event_loop.cpp @@ -61,7 +61,7 @@ LinuxPlatformEventLoop::~LinuxPlatformEventLoop() SPDLOG_CRITICAL("Failed to cleanup epoll"); } -void LinuxPlatformEventLoop::waitForEvents(int timeout) +void LinuxPlatformEventLoop::waitForEventsImpl(int timeout) { const int maxEventCount = 16; epoll_event events[maxEventCount]; diff --git a/src/KDFoundation/platform/linux/linux_platform_event_loop.h b/src/KDFoundation/platform/linux/linux_platform_event_loop.h index d0d69fd..be07762 100644 --- a/src/KDFoundation/platform/linux/linux_platform_event_loop.h +++ b/src/KDFoundation/platform/linux/linux_platform_event_loop.h @@ -35,7 +35,6 @@ class KDFOUNDATION_API LinuxPlatformEventLoop : public AbstractPlatformEventLoop LinuxPlatformEventLoop(LinuxPlatformEventLoop &&other) = delete; LinuxPlatformEventLoop &operator=(LinuxPlatformEventLoop &&other) = delete; - void waitForEvents(int timeout) override; void wakeUp() override; bool registerNotifier(FileDescriptorNotifier *notifier) override; @@ -52,6 +51,9 @@ class KDFOUNDATION_API LinuxPlatformEventLoop : public AbstractPlatformEventLoop int epollEventFromFdMinusType(int fd, FileDescriptorNotifier::NotificationType type); int epollEventFromNotifierTypes(bool read, bool write, bool exception); +protected: + void waitForEventsImpl(int timeout) override; + private: std::unique_ptr createPlatformTimerImpl(Timer *timer) override; diff --git a/src/KDFoundation/platform/macos/macos_platform_event_loop.h b/src/KDFoundation/platform/macos/macos_platform_event_loop.h index cbbe50f..238bf1d 100644 --- a/src/KDFoundation/platform/macos/macos_platform_event_loop.h +++ b/src/KDFoundation/platform/macos/macos_platform_event_loop.h @@ -27,7 +27,6 @@ class KDFOUNDATION_API MacOSPlatformEventLoop : public AbstractPlatformEventLoop MacOSPlatformEventLoop(MacOSPlatformEventLoop &&other) = delete; MacOSPlatformEventLoop &operator=(MacOSPlatformEventLoop &&other) = delete; - void waitForEvents(int timeout) override; void wakeUp() override; bool registerNotifier(FileDescriptorNotifier *notifier) override; @@ -36,6 +35,7 @@ class KDFOUNDATION_API MacOSPlatformEventLoop : public AbstractPlatformEventLoop static void postEmptyEvent(); private: + void waitForEventsImpl(int timeout) override; std::unique_ptr createPlatformTimerImpl(Timer *timer) override; }; diff --git a/src/KDFoundation/platform/macos/macos_platform_event_loop.mm b/src/KDFoundation/platform/macos/macos_platform_event_loop.mm index b14b944..c831ce4 100644 --- a/src/KDFoundation/platform/macos/macos_platform_event_loop.mm +++ b/src/KDFoundation/platform/macos/macos_platform_event_loop.mm @@ -29,7 +29,7 @@ MacOSPlatformEventLoop::~MacOSPlatformEventLoop() = default; -void MacOSPlatformEventLoop::waitForEvents(int timeout) +void MacOSPlatformEventLoop::waitForEventsImpl(int timeout) { @autoreleasepool { NSDate *expiration = [timeout] { diff --git a/src/KDFoundation/platform/win32/win32_platform_event_loop.cpp b/src/KDFoundation/platform/win32/win32_platform_event_loop.cpp index 0c0b9b0..b4b2c6a 100644 --- a/src/KDFoundation/platform/win32/win32_platform_event_loop.cpp +++ b/src/KDFoundation/platform/win32/win32_platform_event_loop.cpp @@ -79,7 +79,7 @@ Win32PlatformEventLoop::~Win32PlatformEventLoop() SPDLOG_WARN("Failed to unregister message window class"); } -void Win32PlatformEventLoop::waitForEvents(int timeout) +void Win32PlatformEventLoop::waitForEventsImpl(int timeout) { MSG msg; bool hasMessage = PeekMessage(&msg, 0, 0, 0, PM_REMOVE); diff --git a/src/KDFoundation/platform/win32/win32_platform_event_loop.h b/src/KDFoundation/platform/win32/win32_platform_event_loop.h index 1de8fcd..89f79c3 100644 --- a/src/KDFoundation/platform/win32/win32_platform_event_loop.h +++ b/src/KDFoundation/platform/win32/win32_platform_event_loop.h @@ -35,8 +35,6 @@ class KDFOUNDATION_API Win32PlatformEventLoop : public AbstractPlatformEventLoop Win32PlatformEventLoop(Win32PlatformEventLoop &&other) = delete; Win32PlatformEventLoop &operator=(Win32PlatformEventLoop &&other) = delete; - void waitForEvents(int timeout) override; - void wakeUp() override; bool registerNotifier(FileDescriptorNotifier *notifier) override; @@ -45,6 +43,7 @@ class KDFOUNDATION_API Win32PlatformEventLoop : public AbstractPlatformEventLoop std::unordered_map timers; private: + void waitForEventsImpl(int timeout) override; std::unique_ptr createPlatformTimerImpl(Timer *timer) override; struct NotifierSet { diff --git a/src/KDGui/platform/android/android_platform_event_loop.cpp b/src/KDGui/platform/android/android_platform_event_loop.cpp index 51e5a92..5ff4918 100644 --- a/src/KDGui/platform/android/android_platform_event_loop.cpp +++ b/src/KDGui/platform/android/android_platform_event_loop.cpp @@ -84,7 +84,7 @@ AndroidPlatformEventLoop::AndroidPlatformEventLoop(AndroidPlatformIntegration *a m_androidPlatformInitegration = androidPlatformInitegration; } -void AndroidPlatformEventLoop::waitForEvents(int timeout) +void AndroidPlatformEventLoop::waitForEventsImpl(int timeout) { android_poll_source *source; if (ALooper_pollAll(timeout, nullptr, nullptr, (void **)&source) >= 0) { diff --git a/src/KDGui/platform/android/android_platform_event_loop.h b/src/KDGui/platform/android/android_platform_event_loop.h index 07ac772..4b59c62 100644 --- a/src/KDGui/platform/android/android_platform_event_loop.h +++ b/src/KDGui/platform/android/android_platform_event_loop.h @@ -36,12 +36,6 @@ class KDGUI_API AndroidPlatformEventLoop : public KDFoundation::AbstractPlatform public: AndroidPlatformEventLoop(AndroidPlatformIntegration *androidPlatformInitegration); - // timeout in msecs - // -1 means wait forever - // 0 means do not wait (i.e. poll) - // +ve number, wait for up to timeout msecs - void waitForEvents(int timeout) override; - // Kick the event loop out of waiting void wakeUp() override; @@ -52,6 +46,7 @@ class KDGUI_API AndroidPlatformEventLoop : public KDFoundation::AbstractPlatform AndroidPlatformIntegration *androidPlatformIntegration(); protected: + void waitForEventsImpl(int timeout) override; std::unique_ptr createPlatformTimerImpl(KDFoundation::Timer *timer) final; private: diff --git a/src/KDGui/platform/linux/wayland/linux_wayland_platform_event_loop.cpp b/src/KDGui/platform/linux/wayland/linux_wayland_platform_event_loop.cpp index 5580cb8..9005434 100644 --- a/src/KDGui/platform/linux/wayland/linux_wayland_platform_event_loop.cpp +++ b/src/KDGui/platform/linux/wayland/linux_wayland_platform_event_loop.cpp @@ -35,7 +35,7 @@ LinuxWaylandPlatformEventLoop::~LinuxWaylandPlatformEventLoop() { } -void LinuxWaylandPlatformEventLoop::waitForEvents(int timeout) +void LinuxWaylandPlatformEventLoop::waitForEventsImpl(int timeout) { auto dpy = m_platformIntegration->display(); auto queue = m_platformIntegration->queue(); @@ -46,7 +46,7 @@ void LinuxWaylandPlatformEventLoop::waitForEvents(int timeout) } // Call the base class to do the actual multiplexing - LinuxPlatformEventLoop::waitForEvents(timeout); + LinuxPlatformEventLoop::waitForEventsImpl(timeout); int fd = wl_display_get_fd(dpy); if (epollEventFromFdPlusType(fd, FileDescriptorNotifier::NotificationType::Read)) { diff --git a/src/KDGui/platform/linux/wayland/linux_wayland_platform_event_loop.h b/src/KDGui/platform/linux/wayland/linux_wayland_platform_event_loop.h index c1c5772..a085013 100644 --- a/src/KDGui/platform/linux/wayland/linux_wayland_platform_event_loop.h +++ b/src/KDGui/platform/linux/wayland/linux_wayland_platform_event_loop.h @@ -28,7 +28,7 @@ class LinuxWaylandPlatformEventLoop : public KDFoundation::LinuxPlatformEventLoo ~LinuxWaylandPlatformEventLoop(); void init(); - void waitForEvents(int timeout) override; + void waitForEventsImpl(int timeout) override; private: std::shared_ptr m_logger; diff --git a/src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.cpp b/src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.cpp index fbfc227..2c51c24 100644 --- a/src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.cpp +++ b/src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.cpp @@ -51,13 +51,13 @@ LinuxXcbPlatformEventLoop::~LinuxXcbPlatformEventLoop() { } -void LinuxXcbPlatformEventLoop::waitForEvents(int timeout) +void LinuxXcbPlatformEventLoop::waitForEventsImpl(int timeout) { // Process any xcb events already waiting for us processXcbEvents(); // Call the base class to do the actual multiplexing - LinuxPlatformEventLoop::waitForEvents(timeout); + LinuxPlatformEventLoop::waitForEventsImpl(timeout); // Now we are awake again, check to see if we have any xcb events to process if (!m_xcbEventsPending) diff --git a/src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.h b/src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.h index 22943e5..f0913e9 100644 --- a/src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.h +++ b/src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.h @@ -32,7 +32,7 @@ class LinuxXcbPlatformEventLoop : public KDFoundation::LinuxPlatformEventLoop explicit LinuxXcbPlatformEventLoop(LinuxXcbPlatformIntegration *platformIntegration); ~LinuxXcbPlatformEventLoop(); - void waitForEvents(int timeout) override; + void waitForEventsImpl(int timeout) override; private: void processXcbEvents(); diff --git a/tests/auto/utils/signal/tst_signal.cpp b/tests/auto/utils/signal/tst_signal.cpp index 3fa7f43..4e8da0d 100644 --- a/tests/auto/utils/signal/tst_signal.cpp +++ b/tests/auto/utils/signal/tst_signal.cpp @@ -49,11 +49,39 @@ TEST_SUITE("Signal") signal2.emit(3); CHECK(val == 4); // val not changing immediately after emit - app.connectionEvaluator()->evaluateDeferredConnections(); + app.processEvents(); CHECK(val == 9); } + TEST_CASE("Emit from another thread") + { + KDFoundation::CoreApplication app; + int mainThreadCounter = 0; + int secondaryThreadCounter = 0; + KDBindings::Signal<> signal; + + std::thread t1([&] { + auto conn = signal.connectDeferred(app.connectionEvaluator(), [&] { + ++mainThreadCounter; + }); + conn = signal.connect([&] { + ++secondaryThreadCounter; + }); + signal.emit(); + assert(secondaryThreadCounter == 1); + }); + + t1.join(); + + REQUIRE(secondaryThreadCounter == 1); + REQUIRE(mainThreadCounter == 0); + + app.processEvents(); + + REQUIRE(mainThreadCounter == 1); + } + TEST_CASE("Emit Multiple Signals with Evaluator") { KDFoundation::CoreApplication app; @@ -85,7 +113,7 @@ TEST_SUITE("Signal") CHECK(val1 == 4); CHECK(val2 == 4); - app.connectionEvaluator()->evaluateDeferredConnections(); + app.processEvents(); CHECK(val1 == 6); CHECK(val2 == 7); @@ -108,7 +136,7 @@ TEST_SUITE("Signal") connection.disconnect(); - app.connectionEvaluator()->evaluateDeferredConnections(); + app.processEvents(); CHECK(val == 4); }