8296654: [macos] Crash when launching JavaFX app with JDK that targets SDK 13

[kevinrushforth]

Apple has changed the behavior for applications that are built using the macOS SDK 13 (which is what XCode 14 uses), such that passing in a null tracking rect to NSView::removeTrackingRect will cause now cause a crash. This has exposed a latent bug in the JavaFX macOS glass code that removes the previous tracking rect even if it is null in setFrame, setFrameSize, and updateTrackingAreas.

The fix is to check that the current tracking rect is non null before calling removeTrackingRect as suggested in both this bug report and duplicate bug JDK-8297131. The latter bug report describes an easy way to reproduce this without building your own JDK, by making a copy of the JDK and modifying the meta-data that indicates the target version of the macOS SDK. I did that, and can reproduce this crash with any JavaFX program without this fix, and verified that it works correctly with this fix.

NOTE: it is the version of the SDK (Xcode) that the JDK is targeted to that matters. It is irrelevant what version of the SDK (i.e., what Xcode) is used to build JavaFX. This make this a more serious bug than it otherwise would be.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8296654: [macos] Crash when launching JavaFX app with JDK that targets SDK 13

Reviewers

• Ajit Ghaisas (@aghaisas - Reviewer)
• Johan Vos (@johanvos - Reviewer)
• Michael Strauß (@mstr2 - Committer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 981

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

Using diff file

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

[bridgekeeper]

👋 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.

[mlbridge]

Webrevs

• 00: Full (dc0d1a69)

[kevinrushforth]

@aghaisas Can you review this?

[aghaisas]

I have a x86_64 Mac system. I was unable to simulate the crash using the smart way described in JDK-8297131. I wonder whether the bug is aarch64 only?

I see that all the occurrences of self->trackingRect in the codebase have null check now.
Approving as it looks the obvious fix.

[openjdk]

@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:

8296654: [macos] Crash when launching JavaFX app with JDK that targets SDK 13

Reviewed-by: aghaisas, jvos, mstrauss

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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.

[johanvos]

I couldn't reproduce a crash either, but the proposed changes look good.

If this were Java code, I would be more cautious as a NullPointer could be catched and lead to different behavior. However, in this ObjectiveC code there is no reasonable flow that would keep working with null references.

[mstr2]

I can reproduce the crash on an M1 Mac, and the proposed change fixes the problem.

[kevinrushforth]

Thanks for the additional reviews. Btw, I was able to reproduce the crash on a Mac x64 system running Ventura, and this fixes it there as well.

[kevinrushforth]

/integrate

[openjdk]

Going to push as commit 5b96d34.

[openjdk]

@kevinrushforth Pushed as commit 5b96d34.

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