Fix Swing dialog deadlock on macOS desktop build#9
Conversation
On macOS, the lwjgl3 desktop build requires -XstartOnFirstThread for GLFW, which means the libGDX render thread is also the AppKit main thread. Swing dialog calls (JFileChooser.showOpenDialog, JOptionPane.showXxxDialog, etc.) were being dispatched onto the libGDX render thread via Gdx.app.postRunnable, which deadlocked because the Swing API normally expects to dispatch through the AWT Event Dispatch Thread, which in turn needs the macOS main thread to be free. DesktopDialogHandler: route the five dialog methods (confirm, openFileDialog, promptForTextInput, showAboutDialog, promptForOption) through SwingUtilities.invokeLater. The response handler is then posted back via Gdx.app.postRunnable so it runs on the libGDX render thread, keeping libGDX state mutations safe. DesktopLauncher: pre-initialise AWT (Toolkit.getDefaultToolkit) before the Lwjgl3Application constructor takes the main thread. Without this, the first Swing call later would try to initialise AppKit from a non-main thread and hang. Pre-initialising claims AppKit before libGDX takes the main thread.
|
@lxpollitt , I have already merged this one but just noticed something that might be related to the change. The dialogs like open file, rom selection dialog and About pop up are opening behind the main JOric window, at least on Windows. I assume it doesn't do that on the Mac? I haven't yet looked into whether there is a way to force the new dialog to appear above the JOric window but I assume there will be a way. |
|
@lanceewing They were in the foreground on macOS. But I did notice they are not modal - you can create multiple of them and you can change window order so they are not the foreground. I thought that was in a sense "existing" behaviour so didn't try to address it. But I can look into forcing the windows to the foreground if you would like me to, no problem. Just let me know. |
|
Let me look into the issue and propose a candidate fix. I won't be able to test it on Windows, so will need your help to validate if it works well enough. |
|
@lanceewing See #18. |
The desktop Swing dialogs (file open, settings, About, and the quit/exit confirmations) could open behind the application window on Windows. This was a behaviour regression introduced by the macOS dialog deadlock fix (#9) which moved the dialog calls onto the AWT event dispatch thread. On Windows a thread that does not own the foreground window cannot bring a new window to the front, so a dialog created on the EDT opens behind the application. Each dialog is now given a temporary, effectively invisible, owner window that is brought to the foreground and disposed as soon as the dialog closes, so the dialog appears in front. By default the code uses a gentle toFront()/requestFocus() to bring this window to the foreground. But a DIALOG_ALWAYS_ON_TOP static flag is included to switch to setting the owner window to always-on-top, which foregrounds more reliably but pins the dialog above all other windows while open, even when switching to ther applications. The correct choise for this static flag needs to be chosen after testing on Windows. The owner window is created on the EDT inside the existing dispatch, so the macOS deadlock fix is preserved.
Problem
On macOS, clicking any UI button that opens a Swing dialog (File Open, Help / About, Emulator Reset, etc.) hangs JOric with a spinning beach ball that never recovers. The in-emulator curated program list still works because it's rendered by libGDX itself rather than via a Swing dialog.
The root cause is the collision of two macOS-specific main-thread constraints:
StartupHelper.startNewJvmIfRequiredalready arranges this by re-launching the child JVM with-XstartOnFirstThread, so the libGDX render thread is the macOS main thread.These two constraints fight: GLFW ends up owning the main thread, leaving no main thread available for AppKit when a Swing dialog needs it. The existing code dispatches Swing dialogs onto the libGDX render thread via
Gdx.app.postRunnable, making the dialog call run on the thread AppKit needs to be free. The dialog parks waiting for the EDT to complete its dispatch; the EDT can't complete because it needs the main thread; the main thread is parked waiting for the dialog - which results in a deadlock.I confirmed this via
jstackon a hung run: the main thread parked inObject.wait()insideJFileChooser.showOpenDialog→WaitDispatchSupport.enter, with the AWT-EventQueue thread spinning insun.lwawt.macosx.LWCToolkit.isApplicationActivetrying to round-trip to the macOS main thread (which was the very thread parked above).I think this is a macOS-only manifestation. Windows and Linux don't have the same
-XstartOnFirstThread-style main-thread constraint for either GLFW or AWT. However, I think the existingGdx.app.postRunnablepattern technically also violates the Swing thread-safety contract on those platforms (Swing components are normally only supposed to be accessed from the EDT), but the violation just happens to not deadlock there because there's no contested main thread to serialise on.Fix
Two changes, both applied unconditionally on all OSes although the bug only manifests on macOS:
DesktopDialogHandler: route the five dialog methods (confirm,openFileDialog,promptForTextInput,showAboutDialog,promptForOption) throughSwingUtilities.invokeLaterinstead ofGdx.app.postRunnable. The response handler is then posted back throughGdx.app.postRunnableso it returns to the libGDX render thread for any subsequent libGDX-state mutation. This is contract-compliant with Swing's "components are only accessed from the EDT" rule on every OS, not just macOS.DesktopLauncher: pre-initialise AWT viajava.awt.Toolkit.getDefaultToolkit()afterStartupHelper.startNewJvmIfRequiredreturns but before theLwjgl3Applicationconstructor takes over the main thread. Without this, the first Swing call later in the lifecycle would try to initialise AppKit from a non-main thread (the dispatched EDT runnable) and hang. Pre-initialising allows AppKit to initialise on the main thread before libGDX takes it over. This is required for macOS and should be no-op-ish on Windows / Linux (where AWT doesn't have a main-thread requirement).Scope
Toolkit.getDefaultToolkit()pre-init is a single line in the launcher. Could be platform-gated to macOS only if preferred; I left it un-gated because the cost on other platforms is negligible and thought it better to have a single code path for simplicity.Testing
I've confirmed the fix behaves as expected for me using the macOS desktop for
openFileDialog,promptForOption,showAboutDialog, andconfirmdialogs. Before this PR, clicking any of these would beach-ball and never recover. After this PR, all these dialogs open promptly and return the results as expected.However, I have not tested the
promptForTextInputdialog because I couldn't see any place in the UI that used it?I also haven't been able to verify Windows / Linux (I don't have easy access to either). The pattern change is conservative enough that I'd expect it to work as a no-op behaviour-wise on those platforms - I assume the dialogs were already working there, and
SwingUtilities.invokeLateris the documented Swing convention. But it would be good to verify on at least one other OS to be sure before merging.