8339178: [macos] Swing InterOp Platform.exit() crash

[kevinrushforth]

trafficstars

A Swing / FX interop app will crash if an application creates a new AWT / Swing window after calling Platform.exit. The root cause of the crash is that AWT caches the JNI env pointer for the AppKit thread, and assumes that it is valid for the life of the application. In the case where JavaFX is the owner of the NSApplication, we detach the AppKit thread from the JVM, after which AWT's env pointer is no longer valid. AWT will therefore crash the next time it does a JNI upcall.

This PR fixes the crash by leaving the macOS AppKit thread attached to the JVM after the JavaFX main event loop terminates. This requires attaching the AppKit thread to the JVM as a daemon thread when JavaFX is the NSApplication owner, matching what AWT does when it is the owner. In order to prevent a JavaFX application from exiting prematurely, create a non-daemon "KeepAlive" thread that can be terminated when the FX toolkit exits. This also solves a somewhat-related problem where the JavaFX toolkit will exit prematurely if the AWT toolkit is started first, and all AWT windows are disposed.

This fix is in addition to the AWT fix: JDK-8190329 / openjdk/jdk#20688. Either the AWT fix or the JavaFX fix is sufficient to avoid this specific problem, but there is value in fixing it in both places, so I cloned the AWT bug to create a JavaFX bug that we can use for this PR.

Summary of the changes:

• Attach the AppKit thread to the JVM as a daemon
• Do not detach the thread when the FX main event loop terminates
• Create and start a KeepAlive thread in MacApplication, when the FX toolkit starts
• Terminate the KeepAlive thread when the FX toolkit finishes

Testing:

I created automated systems tests from the manual test programs in the bugs as well as a test to ensure that we don't regress and exit prematurely in the pure JavaFX case (which would happen without the KeepAlive thread). Two of the new tests fail on macOS without the fix and pass with the fix. All three pass on all platforms with the fix.

---

Progress

• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue
• [ ] Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

• JDK-8339178: [macos] Swing InterOp Platform.exit() crash (Bug - P3)
• JDK-8339183: [macos] Premature exit in Swing interop when last JFrame is disposed (Bug - P4)

Reviewers

• Prasanta Sadhukhan (@prsadhuk - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1545/head:pull/1545
$ git checkout pull/1545

Update a local copy of the PR:
$ git checkout pull/1545
$ git pull https://git.openjdk.org/jfx.git pull/1545/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1545

View PR using the GUI difftool:
$ git pr show -t 1545

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1545.diff

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back kcr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

@kevinrushforth This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8339178: [macos] Swing InterOp Platform.exit() crash
8339183: [macos] Premature exit in Swing interop when last JFrame is disposed

Reviewed-by: psadhukhan, azvegint

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 3 new commits pushed to the master branch:

• e0ceafb7dba15b4faa683c336828e79f19a59d30: 8334124: Rendering issues with CSS "text-shadow" in WebView
• ca70a07b3ee712b1d06baf8a3901e6ae96070124: 8337481: File API: file.name contains path instead of name
• a53bc589ac37d490b1406a2b977097a06bf5ac74: 8339236: Suppress removal warnings for Security Manager methods in iOS sources

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch. As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[kevinrushforth]

/issue add 8339183

[kevinrushforth]

@prsadhuk Can you be one of the reviewers?

/reviewers 2

[openjdk[bot]]

@kevinrushforth Adding additional issue to issue list: 8339183: [macos] Premature exit in Swing interop when last JFrame is disposed.

[openjdk[bot]]

@kevinrushforth The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[mlbridge[bot]]

Webrevs

• 02: Full - Incremental (f2e6561d)
• 01: Full - Incremental (bb4ef9ba)
• 00: Full (61ab3c35)

[kevinrushforth]

/integrate

[openjdk[bot]]

Going to push as commit 465228269ba1d9ad4e902e9f161b79b8a1ebcd81. Since your change was applied there have been 4 commits pushed to the master branch:

• 2bf8ffc411d404be2a3f9107a62ffa7a72fcccbd: 8339212: gradle downloads x64 binary of SWT on Linux/aarch64
• e0ceafb7dba15b4faa683c336828e79f19a59d30: 8334124: Rendering issues with CSS "text-shadow" in WebView
• ca70a07b3ee712b1d06baf8a3901e6ae96070124: 8337481: File API: file.name contains path instead of name
• a53bc589ac37d490b1406a2b977097a06bf5ac74: 8339236: Suppress removal warnings for Security Manager methods in iOS sources

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@kevinrushforth Pushed as commit 465228269ba1d9ad4e902e9f161b79b8a1ebcd81.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.