1
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-06-08 05:27:14 +09:00

LibCore: Make single-shot timer objects manually reset on Windows

This fixes a really nasty EventLoop bug which I debugged for 2 weeks.

The spin_until([&]{return completed_tasks == total_tasks;}) in
TraversableNavigable::check_if_unloading_is_canceled spins forever.

Cause of the bug:

check_if_unloading_is_canceled is called deferred

check_if_unloading_is_canceled creates a task:

        queue_global_task(..., [&] {
            ...
            completed_tasks++;
        }));

This task is never executed.

queue_global_task calls TaskQueue::add

void TaskQueue::add(task)
{
    m_tasks.append(task);
    m_event_loop->schedule();
}

void HTML::EventLoop::schedule()
{
    if (!m_system_event_loop_timer)
        m_system_event_loop_timer = Timer::create_single_shot(
            0, // delay
            [&] { process(); });

    if (!m_system_event_loop_timer->is_active())
        m_system_event_loop_timer->restart();
}

EventLoop::process executes one task from task queue and calls
schedule again if there are more tasks.

So task processing relies on one single-shot zero-delay timer,
m_system_event_loop_timer.

Timers and other notification events are handled by Core::EventLoop
and Core::ThreadEventQueue, these are different from HTML::EventLoop
and HTML::TaskQueue mentioned above.

check_if_unloading_is_canceled is called using deferred_invoke
mechanism, different from m_system_event_loop_timer,
see Navigable::navigate and Core::EventLoop::deferred_invoke.

The core of the problem is that Core::EventLoop::pump is called again
(from spin_until) after timer fired but before its handler is executed.

In ThreadEventQueue::process events are moved into local variable before
executing. The first of those events is check_if_unloading_is_canceled.
One of the rest events is Web::HTML::EventLoop::process sheduled in
EventLoop::schedule using m_system_event_loop_timer.
When check_if_unloading_is_canceled calls queue_global_task its
m_system_event_loop_timer is still active because Timer::timer_event
was not yet called, so the timer is not restarted.
But Timer::timer_event (and hence EventLoop::process) will never execute
because check_if_unloading_is_canceled calls spin_until after
queue_global_task, and EventLoop::process is no longer in
event_queue.m_private->queued_events.

By making a single-shot timer manually-reset we are allowing it to fire
several times. So when spin_until is executed m_system_event_loop_timer
is fired again. Not an ideal solution, but this is the best I could
come up with. This commit makes the behavior match EventLoopImplUnix,
in which single-shot timer can also fire several times.

Adding event_queue.process(); at the start of pump like in EvtLoopImplQt
doesn't fix the problem.

Note: Timer::start calls EventReceiver::start_timer, which calls
EventLoop::register_timer with should_reload always set to true
(single-shot vs periodic are handled in Timer::timer_event instead),
so I use static_cast<Timer&>(object).is_single_shot() instead of
!should_reload.
This commit is contained in:
stasoid 2025-02-19 22:36:05 +05:00 committed by Andrew Kaster
parent 2bfed2e417
commit 17fcbce324
Notes: github-actions[bot] 2025-04-11 01:03:06 +00:00

View file

@ -9,6 +9,7 @@
#include <LibCore/EventLoopImplementationWindows.h>
#include <LibCore/Notifier.h>
#include <LibCore/ThreadEventQueue.h>
#include <LibCore/Timer.h>
#include <AK/Windows.h>
@ -196,7 +197,9 @@ void EventLoopManagerWindows::unregister_notifier(Notifier& notifier)
intptr_t EventLoopManagerWindows::register_timer(EventReceiver& object, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible fire_when_not_visible)
{
VERIFY(milliseconds >= 0);
HANDLE timer = CreateWaitableTimer(NULL, FALSE, NULL);
// FIXME: This is a temporary fix for issue #3641
bool manual_reset = static_cast<Timer&>(object).is_single_shot();
HANDLE timer = CreateWaitableTimer(NULL, manual_reset, NULL);
VERIFY(timer);
LARGE_INTEGER first_time = {};