Fix Swing dialog deadlock on macOS desktop build#9
Open
lxpollitt wants to merge 1 commit into
Open
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.