The main race was possible concurrent access to unordered_map which
could corrupt internal state. Our global variables were also not
atomically updated. DownloadWorkerThread also returned true on error,
which could cause the updater to proceed without all updates being
downloaded which would then fail at the install stage.
With recent changes to the application shutdown logic, events had to
follow a very strict order as certain elements of shutdown code
depend on other elements not being deallocated prematurely.
This turned the (correct) order of events on macOS upside down and
lead to crashes either when the app was quit from within or when
terminated by the OS.
The fix incorporates multiple elements:
* Removal of the custom "Quit" menu item on macOS to use the default
implementation of Qt's platform plugin.
* Soft-revert (via preprocessor conditionals) parts of the updated
shutdown logic to prevent emitting recursive shutdown events.
* Handle main window close event by simply emitting a "quit" event
on the application instance.
* Update POSIX signal handlers to also simply emit a "quit" event.
In combination these changes reduce the number of different code paths
taken during shutdown:
* Closing the app via the menu item, menu item shortcut, or initiated
by AppKit (OS shutdown/reboot, or quit via Dock/Finder) will emit an
AppKit "terminate" event for orderly shutdown.
* Closing the main window or sending an appropriate POSIX signal
triggers the "terminate" event indirectly by emitting the "quit"
event on the application instance.
Either way a "close" event to the main window happens before the
event loop is terminated and the application instance is torn down
(either directly, or indirectly via Qt's "closeAllWindows" function in
response to "terminate"). The order of events thus is always:
0. Terminate event by AppKit (except when closing the main window)
1. Closing of main window
2. Termination of browser docks
3. Deallocation of main window
4. Termination of application
5. Deallocation of application
NOTE: All this only applies to macOS. The shutdown order and procedures
on Windows and Linux are unchanged.
When the main window is closed and with it the application state is torn
down, browser panels need to be explicitly removed before the the CEF
instance used by the application is shut down itself.
For service-based docks this happens as part of the "reset" of the
"auth" pointer (and thus its destructor), for user-created browser
panels this is achieved by the call to "ClearExtraBrowserDocks".
Because the Youtube app dock is a special browser panel that is created
conditionally, but potentially exists globally, it also has to be
closed this way (if it was created).
Otherwise CEF will force-close the underlying browser host instance as
part of its own shutdown and also deallocate the native window used
by the browser. When the QCefWidget then attempts to detach the
native window from the view hierarchy (to avoid this operation from
potentially closing the root window it is anchored to), it will either
attempt to access a CrFatZombie object (and crash) or access deallocated
memory (and also crash).
Richard Stanway