8387684: [PrismFontFile] Possibly unsafe double check locking

[andy-goryachev-oracle]

Fixes double check locking by adding the volatile keyword.

During normal operation, the access seems to happen from two threads:

Thread[#29,QuantumRenderer-0,5,main]
Thread[#34,JavaFX Application Thread,5,main]

On one hand, this access is likely to be mutually exclusive, on the other hand, javafx does allow the nodes to be created in a background thread, and it might be possible to invoke, for example, GlyphLayout.breakRuns() from a background thread.

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

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

Issue

• JDK-8387684: [PrismFontFile] Possibly unsafe double check locking (Bug - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2205

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back angorya! 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]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

The total number of required reviews for this PR has been set to 2 based on the presence of this label: rfr. This can be overridden with the /reviewers command.

[mlbridge]

Webrevs

• 01: Full - Incremental (bab8376f)
• 00: Full (7073e8f4)

[kevinrushforth]

I don't have time to look at this for jfx27, so I recommend leaving it for after the fork, but I see at one other related threading issue. Both should be fixed at the same time (else there is little point in changing anything). The buildSbixStrikeTables() could be called twice, since it isn't inside the inner if (sbixStrikes == null). It won't be called concurrently, but will still do the initialization more than once which is, at best, unnecessary.

Minor: I prefer to leave the = null that you removed, since code is checking / relying on the initial value (yes I know it doesn't actually matter, but I find it more self-documenting with the explicit initialization).

[andy-goryachev-oracle]

Good point, the whole thing is funky. I've restructured the code to make sure it is a proper double check locking and the buildSbixStrikeTables() is called only once.

For the null field, it's not necessary per JLS 4.12.5
https://docs.oracle.com/javase/specs/jls/se26/html/jls-4.html#jls-4.12.5 :

For all reference types (§4.3), the default value is null.

Besides, it annoys me when the debugger steps at each pointless assignments each time.