mirrors/ladybird
1
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-04-20 03:25:13 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

LibThreading: Avert a reference cycle in BackgroundAction

Browse source 
When a BackgroundAction completes, it resolves a Promise (stored on the
BackgroundAction object) with a reference to itself. The Promise will
never unset this resolved value, thus it will hold a strong reference to
the BackgroundAction until it is destroyed. But because the Promise is
owned by the BackgroundAction itself, we have a reference cycle, and
neither object can be destroyed.

The only user of BackgroundAction is the ImageDecoder process. The
consequence was that the ImageDecoder process would never release any
image data for successfully decoded images.

To fix this, instead of storing the promise on the class itself, we can
just create it as a local variable and pass it around.
This commit is contained in:
Timothy Flynn 2025-02-09 13:00:10 -05:00 • committed by Sam Atkins
parent 063a3afe02
commit f20aa82c27
Notes: github-actions[bot] 2025-02-10 16:06:48 +00:00
1 changed files with 14 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
Libraries/LibThreading/BackgroundAction.h
View file

@ -56,12 +56,13 @@ public:
private:
BackgroundAction(ESCAPING Function<ErrorOr<Result>(BackgroundAction&)> action, ESCAPING Function<ErrorOr<void>(Result)> on_complete, ESCAPING Optional<Function<void(Error)>> on_error = {})
: m_promise(Promise::try_create().release_value_but_fixme_should_propagate_errors())
, m_action(move(action))
: m_action(move(action))
, m_on_complete(move(on_complete))
{
auto promise = Promise::construct();
if (m_on_complete) {
m_promise->on_resolution = [](NonnullRefPtr<Core::EventReceiver>& object) -> ErrorOr<void> {
promise->on_resolution = [](NonnullRefPtr<Core::EventReceiver>& object) -> ErrorOr<void> {
auto self = static_ptr_cast<BackgroundAction<Result>>(object);
VERIFY(self->m_result.has_value());
if (auto maybe_error = self->m_on_complete(self->m_result.value()); maybe_error.is_error())
@ -69,24 +70,27 @@ private:
return {};
};
Core::EventLoop::current().add_job(m_promise);
Core::EventLoop::current().add_job(promise);
}
if (on_error.has_value())
m_on_error = on_error.release_value();
enqueue_work([self = NonnullRefPtr(*this), origin_event_loop = &Core::EventLoop::current()]() {
enqueue_work([self = NonnullRefPtr(*this), promise = move(promise), origin_event_loop = &Core::EventLoop::current()]() mutable {
auto result = self->m_action(*self);
// The event loop cancels the promise when it exits.
self->m_canceled |= self->m_promise->is_rejected();
self->m_canceled |= promise->is_rejected();
// All of our work was successful and we weren't cancelled; resolve the event loop's promise.
if (!self->m_canceled && !result.is_error()) {
self->m_result = result.release_value();
// If there is no completion callback, we don't rely on the user keeping around the event loop.
if (self->m_on_complete) {
origin_event_loop->deferred_invoke([self] {
origin_event_loop->deferred_invoke([self, promise = move(promise)] {
// Our promise's resolution function will never error.
(void)self->m_promise->resolve(*self);
(void)promise->resolve(*self);
});
origin_event_loop->wake();
}
@ -96,7 +100,8 @@ private:
if (result.is_error())
error = result.release_error();
self->m_promise->reject(Error::from_errno(ECANCELED));
promise->reject(Error::from_errno(ECANCELED));
if (!self->m_canceled && self->m_on_error) {
origin_event_loop->deferred_invoke([self, error = move(error)]() mutable {
self->m_on_error(move(error));
@ -109,7 +114,6 @@ private:
});
}
NonnullRefPtr<Promise> m_promise;
Function<ErrorOr<Result>(BackgroundAction&)> m_action;
Function<ErrorOr<void>(Result)> m_on_complete;
Function<void(Error)> m_on_error = [](Error error) {