Fix minor bug

[digant73]

Fixed a minor bug two bugs introduced by PR #13876 present since release 0.0.34-17155 Sreported in bug report #16455:
* Fixed crash on firmware installation dialog box when opened starting a game

• Delete Settings dialog box pointer on dialog closure when started with open() method (just to make the code more consistent, not a bug causing crashes). It avoids to demand the closure to the main window on RPCS3 exit

[elad335]

If there is a bug with Qt::WA_DeleteOnClose, I rather it be done to all elements and be wrapped in a utility function.

[Megamouse]

If there is a bug with Qt::WA_DeleteOnClose, I rather it be done to all elements and be wrapped in a utility function.

This is nonsense. We shouldn't add spaghetti code in this project just because one slot fails.
If this is a Qt bug it should be reported in their bug tracker with a minimal example.
Also, as far as I can tell this is a random crash and not deterministic.
So it's not even a guarantee that the workaround works 100%

[digant73]

@elad335 I suspect the Qt message dialogs invoked in OnMissingFw() and InstallPup() could share some properties that origin the bug. Maybe it is only limited to those Qt standard dialogs. I had a look to reference pages of those dialogs but I didn't read anything about any warning on using WA_DeleteOnClose attribute in nested Qt dialogs invokations.
@Megamouse the issue (in terms of sequence of events) is deterministic (the crash is random at 50% as any bug related to access violation). If you put traces in accepted, finished and destroyed connect you will see what I reported. I simply fixed the issue at the moment reporting that reminder in the code so it could be changed once the bug is fixed or maybe the logic in that Qt dialogs is clarified by Qt.
The workaround is working as expected, it is finished and destroyed when it is expected it happens

[elad335]

I suspect the Qt message dialogs invoked in OnMissingFw() and InstallPup() could share some properties that origin the bug. Maybe it is only limited to those Qt standard dialogs. I had a look to reference pages of those dialogs but I didn't read anything about any warning on using WA_DeleteOnClose attribute in nested Qt dialogs invokations.

If we don't understand, not suspect, what's causing it, we replace them all. That is my opinion.

[Megamouse]

There is no indication that deleteonclose is buggy in any other occasion.
Bear in mind that deleteonclose only calls deleteLater under the hood, so there is no real difference. (If exec is used, it uses delete at the end).
For me, this is also not reproducible with some examples I wrote and in rpcs3 it's more like a 10% chance and only in this one dialog.
I also suspect that this wouldn't happen with exec, since that's still the most common way of using message boxes.

[digant73]

Yes I know what should be done internally. I also suspect it is a bug using async dialog, so started with open(), but it is present only if DeleteOnClose is enabled.
See the following code I used for testing. The bug was present also on Qt 6.8.0 (not tested on older Qt versions).

Tested with the following code in OnMissingFw():

	//mb->setAttribute(Qt::WA_DeleteOnClose);

	connect(mb, &QDialog::destroyed, this, []()
	{
		gui_log.error("DESTROYED");
	});

	connect(mb, &QDialog::finished, this, [mb]()
	{
		gui_log.error("FINISHED");
		mb->deleteLater();
	});

	connect(mb, &QDialog::accepted, this, [this]()
	{
		gui_log.error("ACCEPTED - START");
		InstallPup();
		gui_log.error("ACCEPTED - END");
	});

Delete the firmware folders first, then start a game and follow the steps below:

• the first dialog box, in OnMissingFw(), asking to locate and install a firmware is opened.
• press on Locate PS3UPDAT.PUP button to open the second dialog box, in InstallPup().
• press on the Cancel button

and you should always be able to have the following outputs

CORRECT SEQUENCE - Line mb->setAttribute(Qt::WA_DeleteOnClose); commented out (as in this PR and the code above):

E GUI: ACCEPTED - START
E GUI: ACCEPTED - END
E GUI: FINISHED
E GUI: DESTROYED

WRONG SEQUENCE - Line mb->setAttribute(Qt::WA_DeleteOnClose); uncommented (causing often, about 50% in my case, a crash):

E GUI: ACCEPTED - START
E GUI: DESTROYED
E GUI: ACCEPTED - END

[Megamouse]

Open a bugticket in the Qt tracker then 👍

This code seems to work without issues I think (due to having a neglectible runtime in the slot):

	connect(mb, &QDialog::accepted, this, [this]()
	{
		QTimer::singleShot(1, [this]()
		{
			InstallPup();
		});
	});

Anyway, there's an underlying issue with the emulator.
I think the event loop is broken due to some recent changes.
It's pretty much easy to reproduce if I go to the "choose fw file" step 3 times, which leads to qt getting stuck in the getOpenFileName exec call.
If I call OnMissingFw at the beginning in Emu.Load, the thing works without issues.

[digant73]

I also saw that (3 times) but I was thinking it was a problem on my file system after an hard reboot.
Yes, if so, I think there is something more to investigate. Do you have an idea of the recent changes that could broke the event loop?

[Megamouse]

not really. we would have to bisect and see if it's always been an issue 💀 .

[digant73]

I think it could hide many other issues so it is important to identify and fix it.
I will open a bugticket in the Qt tracker. I would leave this PR open just as reminder until, possibly, the bug is fixed by Qt.

The second bug reported in this PR could be provided in one of your next PR

[digant73]

It seems that both the issues on crash and hang (after 3 times invoking InstallPup()) are due to a bug in QFileDialog::getOpenFileName().
An old issue since 2014 https://forum.qt.io/topic/49209/qfiledialog-getopenfilename-hangs-in-windows-when-using-the-native-dialog still valid in 2025. The hang issue was also reported as present in other Qt functions (getSaveFileName(), QFileDialog::getExistingDirectory) also used in RPCS3.

It has been reported by all users that the solution to the hang issue is to not use native dialogs but to use the Qt dialogs (passing the attribute QFileDialog::DontUseNativeDialog).
I tested that on Qt 6.8.1 simply replacing in InstallPup():

QFileDialog::getOpenFileName(this, tr("Select PS3UPDAT.PUP To Install"), path_last_pup, tr("PS3 update file (PS3UPDAT.PUP);;All pup files (*.pup *.PUP);;All files (*.*)"));

with:

QFileDialog::getOpenFileName(this, tr("Select PS3UPDAT.PUP To Install"), path_last_pup, tr("PS3 update file (PS3UPDAT.PUP);;All pup files (*.pup *.PUP);;All files (*.*)"), Q_NULLPTR, QFileDialog::DontUseNativeDialog);

and it is fixing both the anomaly when using Qt::WA_DeleteOnClose (often ending with a crash) and the hang after 3 times InstallPup() is invoked.

Probably a wrapper for those bugged Qt functions, forcing the use of QFileDialog::DontUseNativeDialog, could be a solution while waiting for a solution on Qt side (maybe never). But this is just a choice for the main developers.
Removing the partial fix on this PR.

[Megamouse]

Yeah the native dialogs actually work fine if you set audio to disabled, since we only really use CoInitialize in the audio handlers.

[digant73]

Yes, I removed that partial/not proper fix from this PR. It's up to main developers to provide the fix for that important bug (side effects with those kind of bugs can happen randomly and with other (not only a crash) strange effects (slow down, stuttering etc.)

[Megamouse]

You have to rebase this into one commit or enable "allow maintainers to make changes" or how it's called so I can squash this.

[digant73]

it is already allowed for maintainers to make changes, so you can proceed. Anyway, I do not see any conflict from my side

[Megamouse]

[digant73]

if the branch update is still not working, please provide this minor cleanup PR in one of your PR (there are only two lines changed). No need to waste other time on this PR. I will close it reporting the PR implementing the changes.