Ladybird/Android: Post timer events to the correct event loop

The FIXME at the bottom of Timer.nativeRun was on the money, and was
the cause of some leaked timers. Solve the issue by passing the
EventLoopImplementation to the JVM Timer object and retrieving it's
thread_local timer list, and posting the events to the Impl rather than
trying to invoke the receiver's timer event callback directly in
the Timer thread. Because the implementation of
ALooperEventLoopManager::did_post_event() reaches for
EventLoop::current(), we also need to make sure that the timer thread
acutally *has* an EventLoop, even if we don't expect to use it for
anything. And we need to make sure to pump it to clear out any of the
poke() invocations sending 4 bytes to the wake pipe for that thread.
This commit is contained in:
Andrew Kaster 2023-09-14 15:37:06 -06:00 committed by Andrew Kaster
parent 10d7ec2027
commit ec05b4bc0a
Notes: sideshowbarker 2024-07-18 00:34:07 +09:00
3 changed files with 34 additions and 20 deletions

View file

@ -18,7 +18,7 @@ namespace Ladybird {
EventLoopThreadData& EventLoopThreadData::the() EventLoopThreadData& EventLoopThreadData::the()
{ {
static thread_local EventLoopThreadData s_thread_data; static thread_local EventLoopThreadData s_thread_data { {}, {}, &Core::ThreadEventQueue::current() };
return s_thread_data; return s_thread_data;
} }
@ -73,7 +73,7 @@ int ALooperEventLoopManager::register_timer(Core::EventReceiver& receiver, int m
JavaEnvironment env(m_vm); JavaEnvironment env(m_vm);
auto& thread_data = EventLoopThreadData::the(); auto& thread_data = EventLoopThreadData::the();
auto timer = env.get()->NewObject(m_timer_class, m_timer_constructor, reinterpret_cast<long>(&thread_data)); auto timer = env.get()->NewObject(m_timer_class, m_timer_constructor, reinterpret_cast<long>(&current_impl()));
long millis = milliseconds; long millis = milliseconds;
long timer_id = env.get()->CallLongMethod(m_timer_service, m_register_timer, timer, !should_reload, millis); long timer_id = env.get()->CallLongMethod(m_timer_service, m_register_timer, timer, !should_reload, millis);
@ -112,6 +112,7 @@ void ALooperEventLoopManager::did_post_event()
ALooperEventLoopImplementation::ALooperEventLoopImplementation() ALooperEventLoopImplementation::ALooperEventLoopImplementation()
: m_event_loop(ALooper_prepare(0)) : m_event_loop(ALooper_prepare(0))
, m_thread_data(&EventLoopThreadData::the())
{ {
auto ret = pipe2(m_pipe, O_CLOEXEC | O_NONBLOCK); auto ret = pipe2(m_pipe, O_CLOEXEC | O_NONBLOCK);
VERIFY(ret == 0); VERIFY(ret == 0);
@ -131,6 +132,11 @@ ALooperEventLoopImplementation::~ALooperEventLoopImplementation()
::close(m_pipe[1]); ::close(m_pipe[1]);
} }
EventLoopThreadData& ALooperEventLoopImplementation::thread_data()
{
return *m_thread_data;
}
int ALooperEventLoopImplementation::exec() int ALooperEventLoopImplementation::exec()
{ {
while (!m_exit_requested.load(MemoryOrder::memory_order_acquire)) while (!m_exit_requested.load(MemoryOrder::memory_order_acquire))

View file

@ -44,6 +44,19 @@ private:
jmethodID m_timer_constructor { nullptr }; jmethodID m_timer_constructor { nullptr };
}; };
struct TimerData {
WeakPtr<Core::EventReceiver> receiver;
Core::TimerShouldFireWhenNotVisible visibility;
};
struct EventLoopThreadData {
static EventLoopThreadData& the();
HashMap<long, TimerData> timers;
HashTable<Core::Notifier*> notifiers;
Core::ThreadEventQueue* thread_queue = nullptr;
};
class ALooperEventLoopImplementation : public Core::EventLoopImplementation { class ALooperEventLoopImplementation : public Core::EventLoopImplementation {
public: public:
static NonnullOwnPtr<ALooperEventLoopImplementation> create() { return adopt_own(*new ALooperEventLoopImplementation); } static NonnullOwnPtr<ALooperEventLoopImplementation> create() { return adopt_own(*new ALooperEventLoopImplementation); }
@ -63,6 +76,8 @@ public:
void poke(); void poke();
EventLoopThreadData& thread_data();
private: private:
friend class ALooperEventLoopManager; friend class ALooperEventLoopManager;
@ -77,18 +92,7 @@ private:
int m_pipe[2] {}; int m_pipe[2] {};
int m_exit_code { 0 }; int m_exit_code { 0 };
Atomic<bool> m_exit_requested { false }; Atomic<bool> m_exit_requested { false };
}; EventLoopThreadData* m_thread_data { nullptr };
struct TimerData {
WeakPtr<Core::EventReceiver> receiver;
Core::TimerShouldFireWhenNotVisible visibility;
};
struct EventLoopThreadData {
static EventLoopThreadData& the();
HashMap<long, TimerData> timers;
HashTable<Core::Notifier*> notifiers;
}; };
} }

View file

@ -6,11 +6,16 @@
#include "ALooperEventLoopImplementation.h" #include "ALooperEventLoopImplementation.h"
#include <LibCore/EventLoop.h> #include <LibCore/EventLoop.h>
#include <LibCore/ThreadEventQueue.h>
#include <jni.h> #include <jni.h>
extern "C" JNIEXPORT void JNICALL Java_org_serenityos_ladybird_TimerExecutorService_00024Timer_nativeRun(JNIEnv*, jobject /* thiz */, jlong native_data, jlong id) extern "C" JNIEXPORT void JNICALL
Java_org_serenityos_ladybird_TimerExecutorService_00024Timer_nativeRun(JNIEnv*, jobject /* thiz */, jlong native_data, jlong id)
{ {
auto& thread_data = *reinterpret_cast<Ladybird::EventLoopThreadData*>(native_data); static Core::EventLoop s_event_loop; // Here to exist for this thread
auto& event_loop_impl = *reinterpret_cast<Ladybird::ALooperEventLoopImplementation*>(native_data);
auto& thread_data = event_loop_impl.thread_data();
if (auto timer_data = thread_data.timers.get(id); timer_data.has_value()) { if (auto timer_data = thread_data.timers.get(id); timer_data.has_value()) {
auto receiver = timer_data->receiver.strong_ref(); auto receiver = timer_data->receiver.strong_ref();
@ -21,9 +26,8 @@ extern "C" JNIEXPORT void JNICALL Java_org_serenityos_ladybird_TimerExecutorServ
if (!receiver->is_visible_for_timer_purposes()) if (!receiver->is_visible_for_timer_purposes())
return; return;
Core::TimerEvent event(id); event_loop_impl.post_event(*receiver, make<Core::TimerEvent>(id));
// FIXME: Should the dispatch happen on the thread that registered the timer?
receiver->dispatch_event(event);
} }
// Flush the event loop on this thread to keep any garbage from building up
s_event_loop.pump(Core::EventLoop::WaitMode::PollForEvents);
} }