GraphicPacksWindow2: Disable update button when a game is running

[goeiecool9999]

Constructing DownloadGraphicPacksWindow launches a thread to run the graphic packs update tasks on. When a game is running the thread instead calls wxMessageBox to display an error and immediately returns. On windows this likely uses the system's MessageBox functions, which I believe spawn a window controlled by some system-level process meaning the application only has to read the result. However linux doesn't have native messagebox functionality like that and in order for an application to display any kind of window it has to manage it's event loop. For whatever reason wxMessageBox in the update thread gets stuck and the messagebox cannot be closed. The debugger shows that both the main UI thread and the graphic pack update thread are running a GTK event loop, either waiting for a condition variable or stuck in a poll syscall. This looks like a deadlock, but it's not as when I press the close button on the DownloadGraphicPacksWindow it does actually call OnClose(), which then also locks up the main thread because it tries to join the spawned update thread that is stuck in wxMessageBox. I don't know if this is a bug in wxWidgets or if calling wxwidgets functions from multiple threads just isn't supported. I worked around this issue by changing DownloadGraphicPacksWindow so it doesn't launch the thread upon construction but in ShowModal instead and shows the message box on the main UI thread if a title is running. But then I realised it's kind of bad design to have the button be clickable if it'll immediately tell you it can't perform the action, so I also decided to update the button from FileLoad so it's greyed out when a game starts and made clickable when a game stops. For this I made the new function MainWindow::UpdateChildWindowTitleRunningState in case other windows need similar updates. This effectively means that the message box in DownloadGraphicPacksWindow serves only as a run-time check.

[Exzap]

I think the only rule is that wxWidgets functions have to be called from the wx thread (unless otherwise specified). Did you try moving wxMessageBox out of the update thread and instead into the constructor of DownloadGraphicPacksWindow or into the function where it gets constructed from (OnCheckForUpdates)?

[goeiecool9999]

I think the only rule is that wxWidgets functions have to be called from the wx thread (unless otherwise specified). Did you try moving wxMessageBox out of the update thread and instead into the constructor of DownloadGraphicPacksWindow or into the function where it gets constructed from (OnCheckForUpdates)?

I think it might be a race condition, the thread is launched in the constructor, but the wxMessageBox calls pass the window that's being constructed as their parent. The other wxMessageBox's being called from the thread work just fine now (at least the "no updates" box). Maybe there's still a thread-safety issue but I've not observed freezes or crashes since I changed it to start the thread right before the parent window's ShowModal call.

Not all wxMessageBox calls can be easily moved out of the thread, because not all message boxes indicate success or error status. Some of them indicate intermediate status or ask the user to make a decision.

[goeiecool9999]

Did you try moving wxMessageBox out of the update thread and instead into the constructor of DownloadGraphicPacksWindow or into the function where it gets constructed from (OnCheckForUpdates)

If I moved it into the constructor the check would display a message but I'd have no way of preventing the window from opening. At most I could make it open and immediately close because the thread isn't running. If I moved it to the function in charge of creating the dialog then that places a burden on every function that wants to create DownloadGraphicPacksWindow to check if a title is running or not. That's why I chose to move it to ShowModal instead. But since I disable the button it should never appear in the first place. Unless some future function creates it without checking. I guess the burden isn't completely gone, it's now just safer to do it anyway :sweat_smile: