Close all docs correctly even in case of exception (fix aseprite#3162)

src/app/app.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2021  Igara Studio S.A.
+// Copyright (C) 2018-2022  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -161,7 +161,8 @@ class App::Modules {
   }
 
   ~Modules() {
-    ASSERT(m_recovery == nullptr);
+    ASSERT(m_recovery == nullptr ||
+           ui::get_app_state() == ui::AppState::kClosingWithException);
   }
 
   app::crash::DataRecovery* recovery() {
@@ -368,14 +369,62 @@ int App::initialize(const AppOptions& options)
   return code;
 }
 
+namespace {
+
+#ifdef ENABLE_UI
+  struct CloseMainWindow {
+    std::unique_ptr<MainWindow>& m_win;
+    CloseMainWindow(std::unique_ptr<MainWindow>& win) : m_win(win) { }
+    ~CloseMainWindow() { m_win.reset(nullptr); }
+  };
+#endif
+
+  struct CloseAllDocs {
+    CloseAllDocs() { }
+    ~CloseAllDocs() {
+      auto ctx = UIContext::instance();
+
+      std::vector<Doc*> docs;
+#ifdef ENABLE_UI
+      for (Doc* doc : ctx->getAndRemoveAllClosedDocs())
+        docs.push_back(doc);
+#endif
+      for (Doc* doc : ctx->documents())
+        docs.push_back(doc);
+      for (Doc* doc : docs) {
+        // First we close the document. In this way we receive recent
+        // notifications related to the document as a app::Doc. If
+        // we delete the document directly, we destroy the app::Doc
+        // too early, and then doc::~Document() call
+        // DocsObserver::onRemoveDocument(). In this way, observers
+        // could think that they have a fully created app::Doc when
+        // in reality it's a doc::Document (in the middle of a
+        // destruction process).
+        //
+        // TODO: This problem is because we're extending doc::Document,
+        // in the future, we should remove app::Doc.
+        doc->close();
+        delete doc;
+      }
+    }
+  };
+
+} // anonymous namespace
+
 void App::run()
 {
+#ifdef ENABLE_UI
+  CloseMainWindow closeMainWindow(m_mainWindow);
+#endif
+  CloseAllDocs closeAllDocsAtExit;
+
 #ifdef ENABLE_UI
   // Run the GUI
   if (isGui()) {
+    auto manager = ui::Manager::getDefault();
 #if LAF_WINDOWS
     // How to interpret one finger on Windows tablets.
-    ui::Manager::getDefault()->display()
+    manager->display()
       ->setInterpretOneFingerGestureAsMouseMovement(
         preferences().experimental.oneFingerAsMouseMovement());
 #endif
@@ -442,7 +491,14 @@ void App::run()
 #endif
 
     // Run the GUI main message loop
-    ui::Manager::getDefault()->run();
+    try {
+      manager->run();
+      set_app_state(AppState::kClosing);
+    }
+    catch (...) {
+      set_app_state(AppState::kClosingWithException);
+      throw;
+    }
   }
 #endif  // ENABLE_UI
 
@@ -472,37 +528,6 @@ void App::run()
     m_modules->deleteDataRecovery();
   }
 #endif
-
-  // Destroy all documents from the UIContext.
-  std::vector<Doc*> docs;
-#ifdef ENABLE_UI
-  for (Doc* doc : static_cast<UIContext*>(context())->getAndRemoveAllClosedDocs())
-    docs.push_back(doc);
-#endif
-  for (Doc* doc : context()->documents())
-    docs.push_back(doc);
-  for (Doc* doc : docs) {
-    // First we close the document. In this way we receive recent
-    // notifications related to the document as a app::Doc. If
-    // we delete the document directly, we destroy the app::Doc
-    // too early, and then doc::~Document() call
-    // DocsObserver::onRemoveDocument(). In this way, observers
-    // could think that they have a fully created app::Doc when
-    // in reality it's a doc::Document (in the middle of a
-    // destruction process).
-    //
-    // TODO: This problem is because we're extending doc::Document,
-    // in the future, we should remove app::Doc.
-    doc->close();
-    delete doc;
-  }
-
-#ifdef ENABLE_UI
-  if (isGui()) {
-    // Destroy the window.
-    m_mainWindow.reset(nullptr);
-  }
-#endif
 }
 
 // Finishes the Aseprite application.

src/app/script/events_class.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2021  Igara Studio S.A.
+// Copyright (C) 2021-2022  Igara Studio S.A.
 //
 // This program is distributed under the terms of
 // the End-User License Agreement for Aseprite.
@@ -20,6 +20,7 @@
 #include "app/script/luacpp.h"
 #include "doc/document.h"
 #include "doc/sprite.h"
+#include "ui/app_state.h"
 
 #include <cstring>
 #include <map>
@@ -201,7 +202,7 @@ class SpriteEvents : public Events
 
   ~SpriteEvents() {
     auto doc = this->doc();
-    ASSERT(doc);
+    ASSERT(doc || ui::get_app_state() == ui::AppState::kClosingWithException);
     if (doc) {
       disconnectFromUndoHistory(doc);
       doc->remove_observer(this);

src/ui/CMakeLists.txt

@@ -1,4 +1,5 @@
 # Aseprite UI Library
+# Copyright (C) 2022  Igara Studio S.A.
 # Copyright (C) 2001-2018  David Capello
 
 if(WIN32)
@@ -8,6 +9,7 @@ endif()
 add_library(ui-lib
   accelerator.cpp
   alert.cpp
+  app_state.cpp
   box.cpp
   button.cpp
   combobox.cpp

src/ui/app_state.cpp

@@ -0,0 +1,40 @@
+// Aseprite UI Library
+// Copyright (C) 2022  Igara Studio S.A.
+//
+// This file is released under the terms of the MIT license.
+// Read LICENSE.txt for more information.
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "ui/app_state.h"
+
+#include "ui/manager.h"
+
+namespace ui {
+
+AppState g_state = AppState::kNormal;
+
+void set_app_state(AppState state)
+{
+  g_state = state;
+
+  if (state == AppState::kClosingWithException) {
+    if (auto man = ui::Manager::getDefault())
+      man->_closingAppWithException();
+  }
+}
+
+AppState get_app_state()
+{
+  return g_state;
+}
+
+bool is_app_state_closing()
+{
+  return (g_state == AppState::kClosing ||
+          g_state == AppState::kClosingWithException);
+}
+
+} // namespace ui

src/ui/app_state.h

@@ -0,0 +1,25 @@
+// Aseprite UI Library
+// Copyright (C) 2022  Igara Studio S.A.
+//
+// This file is released under the terms of the MIT license.
+// Read LICENSE.txt for more information.
+
+#ifndef UI_APP_STATE_H_INCLUDED
+#define UI_APP_STATE_H_INCLUDED
+#pragma once
+
+namespace ui {
+
+  enum AppState {
+    kNormal,
+    kClosing,
+    kClosingWithException,
+  };
+
+  void set_app_state(AppState state);
+  AppState get_app_state();
+  bool is_app_state_closing();
+
+} // namespace ui
+
+#endif

src/ui/manager.cpp

@@ -216,20 +216,20 @@ Manager::~Manager()
     // No more cursor
     set_mouse_cursor(kNoCursor);
 
-    // Destroy timers
-    ASSERT(!Timer::haveTimers());
-
-    // Destroy filters
+    // Check timers & filters
 #ifdef _DEBUG
-    for (Filters& msg_filter : msg_filters)
-      ASSERT(msg_filter.empty());
+    if (get_app_state() != AppState::kClosingWithException) {
+      ASSERT(!Timer::haveTimers());
+      for (Filters& msg_filter : msg_filters)
+        ASSERT(msg_filter.empty());
+    }
+    ASSERT(msg_queue.empty());
 #endif
 
     // No more default manager
     m_defaultManager = nullptr;
 
     // Shutdown system
-    ASSERT(msg_queue.empty());
     mouse_widgets_list.clear();
   }
 }
@@ -1089,6 +1089,11 @@ void Manager::dirtyRect(const gfx::Rect& bounds)
   m_dirtyRegion.createUnion(m_dirtyRegion, gfx::Region(bounds));
 }
 
+void Manager::_closingAppWithException()
+{
+  redrawState = RedrawState::ClosingApp;
+}
+
 // Configures the window for begin the loop
 void Manager::_openWindow(Window* window)
 {
@@ -1188,12 +1193,14 @@ void Manager::_closeWindow(Window* window, bool redraw_background)
   // recently closed window).
   updateMouseWidgets(ui::get_mouse_position());
 
-  if (children().empty()) {
-    // All windows were closed...
-    redrawState = RedrawState::ClosingApp;
-  }
-  else {
-    redrawState = RedrawState::AWindowHasJustBeenClosed;
+  if (redrawState != RedrawState::ClosingApp) {
+    if (children().empty()) {
+      // All windows were closed...
+      redrawState = RedrawState::ClosingApp;
+    }
+    else {
+      redrawState = RedrawState::AWindowHasJustBeenClosed;
+    }
   }
 }
 

src/ui/manager.h

@@ -1,5 +1,5 @@
 // Aseprite UI Library
-// Copyright (C) 2018-2021  Igara Studio S.A.
+// Copyright (C) 2018-2022  Igara Studio S.A.
 // Copyright (C) 2001-2017  David Capello
 //
 // This file is released under the terms of the MIT license.
@@ -105,6 +105,7 @@ namespace ui {
     void _openWindow(Window* window);
     void _closeWindow(Window* window, bool redraw_background);
     void _updateMouseWidgets();
+    void _closingAppWithException();
 
   protected:
     bool onProcessMessage(Message* msg) override;

src/ui/ui.h

@@ -1,5 +1,5 @@
 // Aseprite UI Library
-// Copyright (C) 2019-2020  Igara Studio S.A.
+// Copyright (C) 2019-2022  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This file is released under the terms of the MIT license.
@@ -11,6 +11,7 @@
 
 #include "ui/accelerator.h"
 #include "ui/alert.h"
+#include "ui/app_state.h"
 #include "ui/base.h"
 #include "ui/box.h"
 #include "ui/button.h"

src/ui/widget.cpp

@@ -1,5 +1,5 @@
 // Aseprite UI Library
-// Copyright (C) 2018-2021  Igara Studio S.A.
+// Copyright (C) 2018-2022  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This file is released under the terms of the MIT license.
@@ -20,6 +20,7 @@
 #include "os/surface.h"
 #include "os/system.h"
 #include "os/window.h"
+#include "ui/app_state.h"
 #include "ui/init_theme_event.h"
 #include "ui/intern.h"
 #include "ui/layout_io.h"
@@ -687,6 +688,10 @@ Rect Widget::clientChildrenBounds() const
 
 void Widget::setBounds(const Rect& rc)
 {
+  // Don't generate onResize() events if the app is being closed
+  if (is_app_state_closing())
+    return;
+
   ResizeEvent ev(this, rc);
   onResize(ev);
 }

src/ui/window.cpp

@@ -1,5 +1,5 @@
 // Aseprite UI Library
-// Copyright (C) 2018-2020  Igara Studio S.A.
+// Copyright (C) 2018-2022  Igara Studio S.A.
 // Copyright (C) 2001-2017  David Capello
 //
 // This file is released under the terms of the MIT license.
@@ -130,7 +130,8 @@ Window::Window(Type type, const std::string& text)
 
 Window::~Window()
 {
-  manager()->_closeWindow(this, isVisible());
+  if (auto man = manager())
+    man->_closeWindow(this, isVisible());
 }
 
 void Window::setAutoRemap(bool state)
@@ -333,7 +334,8 @@ void Window::closeWindow(Widget* closer)
 
   m_closer = closer;
 
-  manager()->_closeWindow(this, true);
+  if (auto man = manager())
+    man->_closeWindow(this, true);
 
   onClose(ev);
 }