ladybird
ladybird copied to clipboard
LibCore: Single-shot timer is fired twice
There is a delay between firing a timer event and executing a handler for that event. In EventLoopImplementationUnix the situation is possible when event for a single-shot timer is posted to ThreadEventQueue, then handler for another event calls pump again (indirectly through spin_until) and the same timer fires again. I noticed that because on Windows it results in infinite spinning in spin_until called from TraversableNavigable::check_if_unloading_is_canceled, see commit "LibCore: Make single-shot timer objects manually reset on Windows" in #3188 for more info.
After applying this patch the command Meta/ladybird.sh run headless-browser
has this output on Ubuntu 22.04:
patch
diff --git a/Libraries/LibCore/EventLoopImplementationUnix.cpp b/Libraries/LibCore/EventLoopImplementationUnix.cpp
index f323d90bf9..a1acd5b1f0 100644
--- a/Libraries/LibCore/EventLoopImplementationUnix.cpp
+++ b/Libraries/LibCore/EventLoopImplementationUnix.cpp
@@ -16,6 +16,7 @@
#include <LibCore/Socket.h>
#include <LibCore/System.h>
#include <LibCore/ThreadEventQueue.h>
+#include <LibCore/Timer.h>
#include <pthread.h>
#include <sys/select.h>
#include <unistd.h>
@@ -200,6 +201,9 @@ public:
}
}
+ if (zero_delay && ++fire_count >= 2)
+ dbgln("-------- Zero-delay single-shot timer fired {} times", fire_count);
+
// FIXME: While TimerShouldFireWhenNotVisible::Yes prevents the timer callback from being
// called, it doesn't allow event loop to sleep since it needs to constantly check if
// is_visible_for_timer_purposes changed. A better solution will be to unregister a
@@ -215,6 +219,9 @@ public:
WeakPtr<EventReceiver> owner;
pthread_t owner_thread { 0 };
Atomic<bool> is_being_deleted { false };
+
+ bool zero_delay = false;
+ int fire_count = 0;
};
struct ThreadData {
@@ -634,6 +641,9 @@ intptr_t EventLoopManagerUnix::register_timer(EventReceiver& object, int millise
timer->interval = AK::Duration::from_milliseconds(milliseconds);
timer->reload(MonotonicTime::now_coarse());
timer->should_reload = should_reload;
+ timer->zero_delay = milliseconds == 0;
+ if (milliseconds == 0)
+ VERIFY(static_cast<Timer&>(object).is_single_shot());
timer->fire_when_not_visible = fire_when_not_visible;
thread_data.timeouts.schedule_absolute(timer);
return bit_cast<intptr_t>(timer);
diff --git a/Libraries/LibWebView/EventLoop/EventLoopImplementationQt.cpp b/Libraries/LibWebView/EventLoop/EventLoopImplementationQt.cpp
index 18a7fb6bd7..5de3bdc7bc 100644
--- a/Libraries/LibWebView/EventLoop/EventLoopImplementationQt.cpp
+++ b/Libraries/LibWebView/EventLoop/EventLoopImplementationQt.cpp
@@ -198,6 +198,7 @@ int EventLoopImplementationQt::exec()
size_t EventLoopImplementationQt::pump(PumpMode mode)
{
+ dbgln("EventLoopImplementationQt::pump");
auto result = Core::ThreadEventQueue::current().process();
auto qt_mode = mode == PumpMode::WaitForEvents ? QEventLoop::WaitForMoreEvents : QEventLoop::AllEvents;
if (is_main_loop())
diff --git a/Services/WebContent/main.cpp b/Services/WebContent/main.cpp
index 6f10b26c72..ffae45d2ae 100644
--- a/Services/WebContent/main.cpp
+++ b/Services/WebContent/main.cpp
@@ -35,6 +35,7 @@
#include <WebContent/PageClient.h>
#include <WebContent/WebDriverConnection.h>
+#undef HAVE_QT
#if defined(HAVE_QT)
# include <LibWebView/EventLoop/EventLoopImplementationQt.h>
# include <QCoreApplication>
Taking screenshot after 1 seconds
70786.281 WebContent(326424): -------- Zero-delay single-shot timer fired 2 times
70786.281 WebContent(326424): -------- Zero-delay single-shot timer fired 2 times
Saving screenshot to output.png
326415 Lost connection to RequestServer
70787.301 WebContent(326424): Destroying Thread ""(140036729079360) while it is still running undetached!
I propose this fix: add a state "awaiting execution" to the timer, which is set to true when it fired, set to false when it is executed, and don't fire again if "awaiting execution" is true. Change Web::HTML::EventLoop::schedule to create a new timer each time it is called instead of using m_system_event_loop_timer.