Skip to content

Commit 8088588

Browse files
committed
LibCore+LibWeb: Remove the dummy EventReceiver from deferred_invoke()
The DeferredInvocationContext only existed to satisfy the requirement in ThreadEventQueue that each event has an EventReceiver. However, deferred_invoke() was not even using the EventReceiver to call its callback. Therefore, we don't need to allocate one for every deferred invocation. This also prevents WeakPtr::strong_ref() from racing and leaking the context object when invoking a function across threads.
1 parent 4d27e9a commit 8088588

14 files changed

+24
-47
lines changed

Libraries/LibCore/DeferredInvocationContext.h

Lines changed: 0 additions & 20 deletions
This file was deleted.

Libraries/LibCore/Event.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include <AK/Function.h>
1212
#include <AK/Types.h>
1313
#include <AK/WeakPtr.h>
14-
#include <LibCore/DeferredInvocationContext.h>
1514
#include <LibCore/Forward.h>
1615

1716
namespace Core {
@@ -49,15 +48,13 @@ class DeferredInvocationEvent : public Event {
4948
friend class ThreadEventQueue;
5049

5150
public:
52-
DeferredInvocationEvent(NonnullRefPtr<DeferredInvocationContext> context, Function<void()> invokee)
51+
DeferredInvocationEvent(Function<void()>&& invokee)
5352
: Event(Event::Type::DeferredInvoke)
54-
, m_context(move(context))
5553
, m_invokee(move(invokee))
5654
{
5755
}
5856

5957
private:
60-
NonnullRefPtr<DeferredInvocationContext> m_context;
6158
Function<void()> m_invokee;
6259
};
6360

Libraries/LibCore/EventLoop.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ size_t EventLoop::pump(WaitMode mode)
104104

105105
void EventLoop::post_event(EventReceiver& receiver, NonnullOwnPtr<Event>&& event)
106106
{
107-
m_impl->post_event(receiver, move(event));
107+
m_impl->post_event(&receiver, move(event));
108108
}
109109

110110
void EventLoop::add_job(NonnullRefPtr<Promise<NonnullRefPtr<EventReceiver>>> job_promise)
@@ -149,8 +149,7 @@ void EventLoop::wake()
149149

150150
void EventLoop::deferred_invoke(Function<void()> invokee)
151151
{
152-
auto context = DeferredInvocationContext::construct();
153-
post_event(context, make<Core::DeferredInvocationEvent>(context, move(invokee)));
152+
m_impl->post_event(nullptr, make<Core::DeferredInvocationEvent>(move(invokee)));
154153
}
155154

156155
void deferred_invoke(Function<void()> invokee)

Libraries/LibCore/EventLoopImplementation.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class EventLoopImplementation {
5454
virtual void wake() = 0;
5555
virtual bool was_exit_requested() const = 0;
5656

57-
virtual void post_event(EventReceiver& receiver, NonnullOwnPtr<Event>&&) = 0;
57+
virtual void post_event(EventReceiver*, NonnullOwnPtr<Event>&&) = 0;
5858

5959
protected:
6060
EventLoopImplementation();

Libraries/LibCore/EventLoopImplementationUnix.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ class EventLoopTimer final : public EventLoopTimeout {
210210
}
211211
}
212212

213-
ThreadEventQueue::current().post_event(*strong_owner, make<TimerEvent>());
213+
ThreadEventQueue::current().post_event(strong_owner, make<TimerEvent>());
214214
}
215215

216216
AK::Duration interval;
@@ -314,7 +314,7 @@ void EventLoopImplementationUnix::quit(int code)
314314
m_exit_code = code;
315315
}
316316

317-
void EventLoopImplementationUnix::post_event(EventReceiver& receiver, NonnullOwnPtr<Event>&& event)
317+
void EventLoopImplementationUnix::post_event(EventReceiver* receiver, NonnullOwnPtr<Event>&& event)
318318
{
319319
m_thread_event_queue.post_event(receiver, move(event));
320320
if (&m_thread_event_queue != &ThreadEventQueue::current())
@@ -422,7 +422,7 @@ void EventLoopManagerUnix::wait_for_events(EventLoopImplementation::PumpMode mod
422422
type &= notifier.type();
423423

424424
if (type != NotificationType::None)
425-
ThreadEventQueue::current().post_event(notifier, make<NotifierActivationEvent>());
425+
ThreadEventQueue::current().post_event(&notifier, make<NotifierActivationEvent>());
426426
#endif
427427
}
428428
}

Libraries/LibCore/EventLoopImplementationUnix.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#pragma once
88

9+
#include <AK/NonnullOwnPtr.h>
910
#include <AK/Time.h>
1011
#include <LibCore/EventLoopImplementation.h>
1112

@@ -50,7 +51,7 @@ class EventLoopImplementationUnix final : public EventLoopImplementation {
5051

5152
virtual void wake() override;
5253

53-
virtual void post_event(EventReceiver& receiver, NonnullOwnPtr<Event>&&) override;
54+
virtual void post_event(EventReceiver*, NonnullOwnPtr<Event>&&) override;
5455

5556
private:
5657
bool m_exit_requested { false };

Libraries/LibCore/EventLoopImplementationWindows.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ size_t EventLoopImplementationWindows::pump(PumpMode pump_mode)
196196
if (packet->type == CompletionType::Timer) {
197197
auto* timer = static_cast<EventLoopTimer*>(packet);
198198
if (auto owner = timer->owner.strong_ref())
199-
event_queue.post_event(*owner, make<TimerEvent>());
199+
event_queue.post_event(owner, make<TimerEvent>());
200200
if (timer->is_periodic) {
201201
NTSTATUS status = g_system.NtAssociateWaitCompletionPacket(timer->wait_packet.handle, thread_data->iocp.handle, timer->timer.handle, timer, NULL, 0, 0, NULL);
202202
VERIFY(NT_SUCCESS(status));
@@ -205,7 +205,7 @@ size_t EventLoopImplementationWindows::pump(PumpMode pump_mode)
205205
}
206206
if (packet->type == CompletionType::Notifer) {
207207
auto* notifier_data = static_cast<EventLoopNotifier*>(packet);
208-
event_queue.post_event(*notifier_data->notifier, make<NotifierActivationEvent>());
208+
event_queue.post_event(notifier_data->notifier, make<NotifierActivationEvent>());
209209
NTSTATUS status = g_system.NtAssociateWaitCompletionPacket(notifier_data->wait_packet.handle, thread_data->iocp.handle, notifier_data->wait_event.handle, notifier_data, NULL, 0, 0, NULL);
210210
VERIFY(NT_SUCCESS(status));
211211
continue;
@@ -232,7 +232,7 @@ void EventLoopImplementationWindows::quit(int code)
232232
m_exit_code = code;
233233
}
234234

235-
void EventLoopImplementationWindows::post_event(EventReceiver& receiver, NonnullOwnPtr<Event>&& event)
235+
void EventLoopImplementationWindows::post_event(EventReceiver* receiver, NonnullOwnPtr<Event>&& event)
236236
{
237237
m_thread_event_queue.post_event(receiver, move(event));
238238
if (&m_thread_event_queue != &ThreadEventQueue::current())

Libraries/LibCore/EventLoopImplementationWindows.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class EventLoopImplementationWindows final : public EventLoopImplementation {
4242
virtual void wake() override;
4343
virtual bool was_exit_requested() const override { return m_exit_requested; }
4444

45-
virtual void post_event(EventReceiver& receiver, NonnullOwnPtr<Event>&&) override;
45+
virtual void post_event(EventReceiver* receiver, NonnullOwnPtr<Event>&&) override;
4646

4747
private:
4848
bool m_exit_requested { false };

Libraries/LibCore/ThreadEventQueue.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66

77
#include <AK/Vector.h>
8-
#include <LibCore/DeferredInvocationContext.h>
98
#include <LibCore/EventLoopImplementation.h>
109
#include <LibCore/EventReceiver.h>
1110
#include <LibCore/Promise.h>
@@ -22,7 +21,7 @@ struct ThreadEventQueue::Private {
2221
AK_MAKE_DEFAULT_MOVABLE(QueuedEvent);
2322

2423
public:
25-
QueuedEvent(EventReceiver& receiver, NonnullOwnPtr<Event> event)
24+
QueuedEvent(RefPtr<EventReceiver> const& receiver, NonnullOwnPtr<Event> event)
2625
: receiver(receiver)
2726
, event(move(event))
2827
{
@@ -67,7 +66,7 @@ ThreadEventQueue::ThreadEventQueue()
6766

6867
ThreadEventQueue::~ThreadEventQueue() = default;
6968

70-
void ThreadEventQueue::post_event(Core::EventReceiver& receiver, NonnullOwnPtr<Core::Event> event)
69+
void ThreadEventQueue::post_event(Core::EventReceiver* receiver, NonnullOwnPtr<Core::Event> event)
7170
{
7271
{
7372
Threading::MutexLocker lock(m_private->mutex);
@@ -106,16 +105,16 @@ size_t ThreadEventQueue::process()
106105
auto receiver = queued_event.receiver.strong_ref();
107106
auto& event = *queued_event.event;
108107

109-
if (!receiver) {
108+
if (event.type() == Event::Type::DeferredInvoke) {
109+
static_cast<DeferredInvocationEvent&>(event).m_invokee();
110+
} else if (!receiver) {
110111
switch (event.type()) {
111112
case Event::Quit:
112113
VERIFY_NOT_REACHED();
113114
default:
114115
// Receiver disappeared, drop the event on the floor.
115116
break;
116117
}
117-
} else if (event.type() == Event::Type::DeferredInvoke) {
118-
static_cast<DeferredInvocationEvent&>(event).m_invokee();
119118
} else {
120119
NonnullRefPtr<EventReceiver> protector(*receiver);
121120
receiver->dispatch_event(event);

Libraries/LibCore/ThreadEventQueue.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <AK/NonnullOwnPtr.h>
1010
#include <AK/OwnPtr.h>
11+
#include <LibCore/Forward.h>
1112

1213
namespace Core {
1314

@@ -25,7 +26,7 @@ class ThreadEventQueue {
2526
size_t process();
2627

2728
// Posts an event to the event queue.
28-
void post_event(EventReceiver& receiver, NonnullOwnPtr<Event>);
29+
void post_event(EventReceiver*, NonnullOwnPtr<Event>);
2930

3031
// Used by Threading::BackgroundAction.
3132
void add_job(NonnullRefPtr<Promise<NonnullRefPtr<EventReceiver>>>);

0 commit comments

Comments
 (0)