From 4d2357f8f303673aa2d9d980fb877d7c061e85fe Mon Sep 17 00:00:00 2001 From: Itamar Date: Fri, 18 Feb 2022 17:18:22 +0200 Subject: [PATCH] HackStudio: Don't store a global RefPtr to the HackStudioWidget Previously, we stored a RefPtr to the HackStudioWidget in the global scope. This led to a destruction-order related use-after-free bug, where the global HackStudioWidget instance destructed after the static-local GUI::Clipboard instance. When HackStudioWidget destructs it attempts to use the global Clipboard instance, which had already been freed. This caused the Hack Studio process to spin endlessly on exit because it attempted to access the HashTable of the freed Clipboard object. We now store a global WeakPtr to the HackStudioWidget instead, and limit the lifetime of the object to the main function scope. --- Userland/DevTools/HackStudio/main.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/Userland/DevTools/HackStudio/main.cpp b/Userland/DevTools/HackStudio/main.cpp index d2c71ba06d..f3ac2e8452 100644 --- a/Userland/DevTools/HackStudio/main.cpp +++ b/Userland/DevTools/HackStudio/main.cpp @@ -27,7 +27,7 @@ using namespace HackStudio; -static RefPtr s_hack_studio_widget; +static WeakPtr s_hack_studio_widget; static bool make_is_available(); static void notify_make_not_available(); @@ -63,24 +63,25 @@ ErrorOr serenity_main(Main::Arguments arguments) if (argument_absolute_path.is_null() || mode_coredump) project_path = Core::File::real_path_for("."); - s_hack_studio_widget = TRY(window->try_set_main_widget(project_path)); + auto hack_studio_widget = TRY(window->try_set_main_widget(project_path)); + s_hack_studio_widget = hack_studio_widget; - window->set_title(String::formatted("{} - Hack Studio", s_hack_studio_widget->project().name())); + window->set_title(String::formatted("{} - Hack Studio", hack_studio_widget->project().name())); - s_hack_studio_widget->initialize_menubar(*window); + hack_studio_widget->initialize_menubar(*window); window->on_close_request = [&]() -> GUI::Window::CloseRequestDecision { - s_hack_studio_widget->locator().close(); - if (s_hack_studio_widget->warn_unsaved_changes("There are unsaved changes, do you want to save before exiting?") == HackStudioWidget::ContinueDecision::Yes) + hack_studio_widget->locator().close(); + if (hack_studio_widget->warn_unsaved_changes("There are unsaved changes, do you want to save before exiting?") == HackStudioWidget::ContinueDecision::Yes) return GUI::Window::CloseRequestDecision::Close; return GUI::Window::CloseRequestDecision::StayOpen; }; window->show(); - s_hack_studio_widget->update_actions(); + hack_studio_widget->update_actions(); if (mode_coredump) - s_hack_studio_widget->open_coredump(argument_absolute_path); + hack_studio_widget->open_coredump(argument_absolute_path); return app->exec(); }