LibCore: Make Timers and Notifiers aware of threads

Previously sharing a Timer/Notifier between threads (or just handing
its ownership to another thread) lead to a crash as they are
thread-specific.
This commit makes it so we can handle mutation (i.e. just deletion
or unregistering) in a thread-safe and lockfree manner.
This commit is contained in:
Ali Mohammad Pur
2024-05-08 20:10:00 +02:00
committed by Andreas Kling
parent 5cc90f848f
commit 96c7e83345
2 changed files with 49 additions and 10 deletions

View File

@@ -16,6 +16,7 @@
#include <LibCore/Socket.h> #include <LibCore/Socket.h>
#include <LibCore/System.h> #include <LibCore/System.h>
#include <LibCore/ThreadEventQueue.h> #include <LibCore/ThreadEventQueue.h>
#include <pthread.h>
#include <sys/select.h> #include <sys/select.h>
#include <unistd.h> #include <unistd.h>
@@ -25,7 +26,10 @@ namespace {
struct ThreadData; struct ThreadData;
class TimeoutSet; class TimeoutSet;
thread_local ThreadData* s_thread_data; HashMap<pthread_t, ThreadData*> s_thread_data;
static pthread_rwlock_t s_thread_data_lock_impl;
static pthread_rwlock_t* s_thread_data_lock = nullptr;
thread_local pthread_t s_thread_id;
short notification_type_to_poll_events(NotificationType type) short notification_type_to_poll_events(NotificationType type)
{ {
@@ -214,16 +218,41 @@ public:
bool should_reload { false }; bool should_reload { false };
TimerShouldFireWhenNotVisible fire_when_not_visible { TimerShouldFireWhenNotVisible::No }; TimerShouldFireWhenNotVisible fire_when_not_visible { TimerShouldFireWhenNotVisible::No };
WeakPtr<EventReceiver> owner; WeakPtr<EventReceiver> owner;
pthread_t owner_thread { 0 };
Atomic<bool> is_being_deleted { false };
}; };
struct ThreadData { struct ThreadData {
static ThreadData& the() static ThreadData& the()
{ {
if (!s_thread_data) { if (!s_thread_data_lock) {
// FIXME: Don't leak this. pthread_rwlock_init(&s_thread_data_lock_impl, nullptr);
s_thread_data = new ThreadData; s_thread_data_lock = &s_thread_data_lock_impl;
} }
return *s_thread_data;
if (s_thread_id == 0)
s_thread_id = pthread_self();
ThreadData* data = nullptr;
pthread_rwlock_rdlock(&*s_thread_data_lock);
if (!s_thread_data.contains(s_thread_id)) {
// FIXME: Don't leak this.
data = new ThreadData;
pthread_rwlock_unlock(&*s_thread_data_lock);
pthread_rwlock_wrlock(&*s_thread_data_lock);
s_thread_data.set(s_thread_id, data);
} else {
data = s_thread_data.get(s_thread_id).value();
}
pthread_rwlock_unlock(&*s_thread_data_lock);
return *data;
}
static ThreadData& for_thread(pthread_t thread_id)
{
pthread_rwlock_rdlock(&*s_thread_data_lock);
auto& result = *s_thread_data.get(thread_id).value();
pthread_rwlock_unlock(&*s_thread_data_lock);
return result;
} }
ThreadData() ThreadData()
@@ -610,6 +639,7 @@ intptr_t EventLoopManagerUnix::register_timer(EventReceiver& object, int millise
VERIFY(milliseconds >= 0); VERIFY(milliseconds >= 0);
auto& thread_data = ThreadData::the(); auto& thread_data = ThreadData::the();
auto timer = new EventLoopTimer; auto timer = new EventLoopTimer;
timer->owner_thread = s_thread_id;
timer->owner = object; timer->owner = object;
timer->interval = Duration::from_milliseconds(milliseconds); timer->interval = Duration::from_milliseconds(milliseconds);
timer->reload(MonotonicTime::now_coarse()); timer->reload(MonotonicTime::now_coarse());
@@ -621,11 +651,14 @@ intptr_t EventLoopManagerUnix::register_timer(EventReceiver& object, int millise
void EventLoopManagerUnix::unregister_timer(intptr_t timer_id) void EventLoopManagerUnix::unregister_timer(intptr_t timer_id)
{ {
auto& thread_data = ThreadData::the();
auto* timer = bit_cast<EventLoopTimer*>(timer_id); auto* timer = bit_cast<EventLoopTimer*>(timer_id);
if (timer->is_scheduled()) auto& thread_data = ThreadData::for_thread(timer->owner_thread);
thread_data.timeouts.unschedule(timer); auto expected = false;
delete timer; if (timer->is_being_deleted.compare_exchange_strong(expected, true, AK::MemoryOrder::memory_order_acq_rel)) {
if (timer->is_scheduled())
thread_data.timeouts.unschedule(timer);
delete timer;
}
} }
void EventLoopManagerUnix::register_notifier(Notifier& notifier) void EventLoopManagerUnix::register_notifier(Notifier& notifier)
@@ -639,11 +672,13 @@ void EventLoopManagerUnix::register_notifier(Notifier& notifier)
.events = notification_type_to_poll_events(notifier.type()), .events = notification_type_to_poll_events(notifier.type()),
.revents = 0, .revents = 0,
}); });
notifier.set_owner_thread(s_thread_id);
} }
void EventLoopManagerUnix::unregister_notifier(Notifier& notifier) void EventLoopManagerUnix::unregister_notifier(Notifier& notifier)
{ {
auto& thread_data = ThreadData::the(); auto& thread_data = ThreadData::for_thread(notifier.owner_thread());
auto it = thread_data.notifier_by_ptr.find(&notifier); auto it = thread_data.notifier_by_ptr.find(&notifier);
VERIFY(it != thread_data.notifier_by_ptr.end()); VERIFY(it != thread_data.notifier_by_ptr.end());

View File

@@ -32,11 +32,15 @@ public:
void event(Core::Event&) override; void event(Core::Event&) override;
void set_owner_thread(pthread_t owner_thread) { m_owner_thread = owner_thread; }
pthread_t owner_thread() const { return m_owner_thread; }
private: private:
Notifier(int fd, Type type, EventReceiver* parent = nullptr); Notifier(int fd, Type type, EventReceiver* parent = nullptr);
int m_fd { -1 }; int m_fd { -1 };
bool m_is_enabled { false }; bool m_is_enabled { false };
pthread_t m_owner_thread { 0 };
Type m_type { Type::None }; Type m_type { Type::None };
}; };