frontend: Adjust application shutdown logic #12668

Warchamp7 · 2025-09-26T01:29:46Z

Description

This PR does a bit of clean up and separation of logic in OBSBasic's shutdown process and makes a number of improvements to how OBS handles being closed by things other than a normal shutdown.

Removes the app shutdown in response to Qt's commitDataRequest in favor of only saving data
Removes the crash handler sentinel file earlier in the shutdown process.
- It is now done after the main window is closed, which is after OBS has done most if it's own cleanup, but before the application is fully shutdown. Off thread discussion led to this decision as OBS' data is saved at this point, and plugins misbehaving at this point is not really a helpful reason to trigger the unclean shutdown dialog.
On Windows, sets shutdown priority for OBS to be higher
- This lets OBS respond to WM_QUERYENDSESSION before Windows kills off the browser subprocesses, as normally they'd be terminated at the same step.
- This fixes a crash in the browser source when forcing a log off / shutdown with active outputs
On Windows, adds explicit handling for WM_QUERYENDSESSION and WM_ENDSESSION which are sent during the log out process to inform apps the session is ending, and allow them to stall the process.

I have tried to keep my changes to the shutdown flow as minimal as possible, as it is particularly delicate and very sensitive to the order of operations. Testing on all operating systems will be valuable.

Motivation and Context

We've received a lot of great feedback on the Unclean Shutdown dialog after we removed the ability to suppress it. We believe that this dialog serves an important purpose but only when it is being shown accurately. This PR aims to address a majority of the cases where it was showing up unexpected or unintended.

How Has This Been Tested?

Painstakingly signing out of Windows multiple times with OBS running in various states.

Enabled the warning for confirmation on exit with outputs active. Ran OBS with no outputs active, with virtual camera active, and with virtual camera active then clicking Exit to display the confirmation prompt prior to log off. Then tested signing out in all scenarios, and both forcing the sign out to continue, or cancelling it.

When sign out is cancelled with outputs active, OBS stays running and displays the confirmation dialog if it was not showing already. When sign out continues, OBS starts normally the following time with no unclean shutdown warning.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Tweak (non-breaking change to improve existing functionality)

Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

My code has been run through clang-format.
I have read the contributing document.
My code is not on the master branch.
The code has been tested.
All commit messages are properly formatted and commits squashed where appropriate.
I have included updates to all appropriate documentation.

frontend/widgets/OBSBasic.cpp

derrod · 2025-09-26T09:25:44Z

frontend/OBSApp.cpp

 	connect(snInt, &QSocketNotifier::activated, this, &OBSApp::ProcessSigInt);
 #else
-	connect(qApp, &QGuiApplication::commitDataRequest, this, &OBSApp::commitData);
+	connect(qApp, &QGuiApplication::commitDataRequest, this, &OBSApp::commitData, Qt::DirectConnection);


This is also just catching WM_QUERYENDSESSION so it could be removed unless it does anything useful on other platforms.

I haven't found out how Qt behaves on Linux, at least no Linux-specific platform plugin seems to call commitData or aboutToQuit as Windows or macOS do.

At least under xcb Qt uses its session manager.

On macOS commitData should occur in reaction to - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication *)sender, which serves the same purpose as the WM_QUERYENDSESSION signal (allowing apps to cancel the shutdown procedure, with the expectation that they also signal when they took care of stuff and are actually able to comply).

This is also just catching WM_QUERYENDSESSION so it could be removed unless it does anything useful on other platforms.

Qt's documentation suggests that we ourselves should not try to exit in response to commitDataRequest, it should only save data. The session manager may try to quit() the application later in the process, depending how things respond.

https://doc.qt.io/qt-6/qguiapplication.html#commitDataRequest

This PR also adjusted commitData() to only save data, I think at worst this is an extra safety.

frontend/OBSApp.cpp

frontend/OBSApp.hpp

PatTheMav · 2025-09-26T12:37:18Z

frontend/OBSApp.cpp

 	connect(snInt, &QSocketNotifier::activated, this, &OBSApp::ProcessSigInt);
 #else
-	connect(qApp, &QGuiApplication::commitDataRequest, this, &OBSApp::commitData);
+	connect(qApp, &QGuiApplication::commitDataRequest, this, &OBSApp::commitData, Qt::DirectConnection);


I haven't found out how Qt behaves on Linux, at least no Linux-specific platform plugin seems to call commitData or aboutToQuit as Windows or macOS do.

At least under xcb Qt uses its session manager.

On macOS commitData should occur in reaction to - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication *)sender, which serves the same purpose as the WM_QUERYENDSESSION signal (allowing apps to cancel the shutdown procedure, with the expectation that they also signal when they took care of stuff and are actually able to comply).

frontend/widgets/OBSBasic.cpp

frontend/OBSApp.cpp

Chiitoo · 2025-10-10T15:26:06Z

Testing this on Gentoo Linux, it seems to be working fine, but I don't think I ever got to see the "unsafe shutdown detected" or so, even without it.

I have a way to prod a segmentation fault even (I promies I'll eventually actually report that one), and OBS Studio still starts just fine.

I haven't looked at the code, but I don't think it should be anything related to 'systemd' or something like that which I don't use, so it is a bit interesting to me.

It might be interesting if people who /do/ get the pop-up/logs for un-clean shutdowns explain how exactly they do get them.

In addition to the segfault, I've been sending OBS Studio the various signals ranging from 1 to 15, such as kill (9) and terminate (15) and nothing seems to prod up the message.

On that note, I think most if not all desktop environments will/should send the term (15) signal to all applications when shutting down, though I saw some discussion of 'systemd' also sending 'kill' (9) immediately after, but they may have made that better some time ago (I have no systems using 'systemd' to test right now).

In any case, thank you!

Dankirk · 2025-10-14T05:16:20Z

There is a potential application hang when experimental captions are enabled and trying to stop it. Fix candidate in #12729

PatTheMav

At least on macOS these changes seem to dangerously interfere with the orderly shutdown process of the app:

When OBS is closed from the outside (by the OS), then
1. commitData is called
2. saveAll is called
3. closeWindow is called via closeEvent
4. saveAll is called again`
5. OBSBasic's destructor runs (triggered by a QEvent::DeferredDelete event)
6. The destructor crashes
When OBS is closing itself, then
1. closeWindow is called
2. saveAll is called
3. commitData is called
4. saveAll is called again`
5. closeWindow is called again (triggered by QApplication::aboutToQuit), but bails out early
6. OBSBasic's destructor runs

Here's what STDERR gave me:

info: [OBSBasic] Destructor
libc++abi: terminating due to uncaught exception of type std::__1::system_error: mutex lock failed: Invalid argument
warning: QMetaObject::invokeMethod: No such method QObject::DoCefMessageLoop(int)

Confusingly enough the mutex in question is the log_mutex used by too_many_repeated_entries after the info message has been printed.

PatTheMav · 2025-10-23T16:05:19Z

frontend/widgets/OBSBasic.cpp

+	applicationShutdown();
+	deleteLater();
+
+	App()->quit();


This single line will make OBS crash on macOS if the application is closed by the OS either because of a shutdown/restart or the user quit the application using the dock icon.

On macOS, the external termination message is handled by the global application instance, which already calls QApplication::Quit as part of its platform plugin, which will send the close event to the main window all by itself.

Calling it again here leads to a nested call to Quit, which seems to have undefined side-effects.

This review thread is unresolved.

m1neral-rocks · 2025-10-26T20:24:53Z

@Warchamp7

Have you tested the above linked PR that is aimed at addressing the erroneous crash issues?

Sorry for pinging you twice. The other one got clogged by Joel's ego. My question was:

I started getting cmake errors a few days ago, it was compiling fine before. Is there an issue with dependencies relating to Qt::GuiPrivate or did I mess something up?

I want to try this pr but I feel like I'm missing something? I wasn't getting this dependency issue before.

Warchamp7 · 2025-10-26T23:09:31Z

@Warchamp7

Have you tested the above linked PR that is aimed at addressing the erroneous crash issues?

Sorry for pinging you twice. The other one got clogged by Joel's ego. My question was:

I started getting cmake errors a few days ago, it was compiling fine before. Is there an issue with dependencies relating to Qt::GuiPrivate or did I mess something up?

I want to try this pr but I feel like I'm missing something? I wasn't getting this dependency issue before.

You can download artifacts compiled on CI of this PR:
https://github.com/obsproject/obs-studio/actions/runs/18130631287?pr=12668#artifacts

PatTheMav

Most recent pushed fixed the crash bug on macOS. Indeed QGuiApplicationPrivate::quit never seems to be executed at all (neither when terminated via macOS nor when quitting the app directly), probably the app is torn down before the invocation can take place.

PatTheMav · 2025-11-03T17:58:17Z

frontend/utility/NativeEventFilter.cpp

@@ -0,0 +1,65 @@
+


Missing GPL header.

PatTheMav · 2025-11-03T17:59:03Z

frontend/utility/NativeEventFilter.cpp

@@ -0,0 +1,65 @@
+
+
+#include <utility/NativeEventFilter.hpp>


Suggested change

#include <utility/NativeEventFilter.hpp>

#include "NativeEventFilter.hpp"

PatTheMav · 2025-11-03T17:59:40Z

frontend/utility/NativeEventFilter.cpp

+
+bool NativeEventFilter::nativeEventFilter(const QByteArray &eventType, void *message, qintptr *result)
+{
+	if (eventType == "windows_generic_MSG") {


Is "windows_generic_MSG" even a possible event type outside of Windows?

It's not, but ifdefing the entire block seemed odd to me, as this method is platform agnostic

https://doc.qt.io/qt-6/qabstractnativeeventfilter.html#nativeEventFilter

That's why in their own implementations they use a generic header, but platform-specific implementations, because then you don't get weird constructs like these were the source code is sliced and diced by preprocessor macros.

It's also the approach I advise to use generally. In this case you would add NativeEventFilter_Windows.cpp, NativeEventFilter_MacOs.cpp, etc. which at the very least implement stubs to implement the full object.

The compiler will not care: If the functions return immediately or via analysis are proven to not do anything, it will not even emit a function call. But for us puny humans there will be cleanly separated implementations for each platform where the internal code is free to be custom-tailored to its platform and doesn't have to contort itself to somehow support different ones in the same implementation.

The platform-specific CrasHandler implementations follow that principle.

PatTheMav · 2025-11-03T18:00:14Z

frontend/utility/NativeEventFilter.hpp

@@ -0,0 +1,12 @@
+#pragma once


Missing GPL header.

Warchamp7 added Bug Fix Non-breaking change which fixes an issue Windows Affects Windows macOS Affects macOS Linux Affects Linux UI/UX Anything to do with changes or additions to UI/UX elements. labels Sep 26, 2025

notr1ch reviewed Sep 26, 2025

View reviewed changes

frontend/widgets/OBSBasic.cpp Show resolved Hide resolved

notr1ch reviewed Sep 26, 2025

View reviewed changes

frontend/widgets/OBSBasic.cpp Show resolved Hide resolved

WizardCM added the Seeking Testers Build artifacts on CI label Sep 26, 2025

Warchamp7 mentioned this pull request Sep 26, 2025

Removal of --disable-shutdown-check breaks automated background workflows #12650

Closed

Warchamp7 force-pushed the windows-shutdown branch from 3a8a6a9 to f97a730 Compare September 26, 2025 02:25

derrod reviewed Sep 26, 2025

View reviewed changes

PatTheMav reviewed Sep 26, 2025

View reviewed changes

Warchamp7 force-pushed the windows-shutdown branch 5 times, most recently from 6d08792 to 617e7ea Compare September 30, 2025 12:56

PatTheMav reviewed Sep 30, 2025

View reviewed changes

frontend/widgets/OBSBasic.cpp Show resolved Hide resolved

frontend/widgets/OBSBasic.cpp Outdated Show resolved Hide resolved

frontend/widgets/OBSBasic.cpp Outdated Show resolved Hide resolved

frontend/OBSApp.cpp Show resolved Hide resolved

frontend/OBSApp.cpp Outdated Show resolved Hide resolved

PatTheMav added this to OBS Studio 32.1 PR Considerations Oct 8, 2025

PatTheMav moved this to In Review in OBS Studio 32.1 PR Considerations Oct 8, 2025

Fenrirthviti mentioned this pull request Oct 12, 2025

[Windows 11] OBS crashes during regular Windows shutdown #10848

Open

PatTheMav reviewed Oct 23, 2025

View reviewed changes

Warchamp7 force-pushed the windows-shutdown branch from 617e7ea to 6b2debe Compare October 31, 2025 02:42

RytoEX requested review from PatTheMav and derrod October 31, 2025 19:26

RytoEX requested a review from notr1ch October 31, 2025 19:26

Warchamp7 added 2 commits October 31, 2025 20:44

frontend: Adjust application shutdown logic

a6ce9e1

squashme: Keep quit as invoked method

a0930bd

Warchamp7 force-pushed the windows-shutdown branch from e06d69a to a0930bd Compare November 1, 2025 00:44

PatTheMav reviewed Nov 3, 2025

View reviewed changes

RytoEX added this to the OBS Studio 32.0 milestone Nov 6, 2025

Warchamp7 mentioned this pull request Nov 9, 2025

Fenrirthviti was wrong beyond reason. Removal of --disable-shutdown-check breaks automated background workflow. See #12650. #12801

Closed

Warchamp7 force-pushed the windows-shutdown branch from ec4ceb3 to 05047d3 Compare November 11, 2025 22:05

squashme: Review fixes

756d5fb

Warchamp7 force-pushed the windows-shutdown branch from 05047d3 to 756d5fb Compare November 11, 2025 22:34

	#include <utility/NativeEventFilter.hpp>
	#include "NativeEventFilter.hpp"

Uh oh!

frontend: Adjust application shutdown logic #12668

Are you sure you want to change the base?

frontend: Adjust application shutdown logic #12668

Conversation

Warchamp7 commented Sep 26, 2025

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Warchamp7 Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Chiitoo commented Oct 10, 2025

Uh oh!

Dankirk commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PatTheMav left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PatTheMav Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m1neral-rocks commented Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Warchamp7 commented Oct 26, 2025

Uh oh!

PatTheMav left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PatTheMav Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Warchamp7 Sep 27, 2025 •

edited

Loading

Dankirk commented Oct 14, 2025 •

edited

Loading

PatTheMav left a comment •

edited

Loading

PatTheMav Oct 23, 2025 •

edited

Loading

m1neral-rocks commented Oct 26, 2025 •

edited

Loading

PatTheMav Nov 5, 2025 •

edited

Loading