RFR: JDK-8269921 Text in Textflow and listeners on bounds can cause endless loop/crash and other performance issues [v3]

[Michael Strauß]

On Fri, 5 Aug 2022 07:34:43 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origjfx/master' into JDK-8269921-textflow-bug
>>  - JDK-8269921
>>    Added a copyright header
>>  - JDK-8269921
>>    Fixing a crash related to Text and TextFlow with bounds listeners
>
> I got this problem as well today. NPE as `runs` is null in line `359`. Does it make sense to first 'resolve' this by adding a simple null check and later we may try your other change which removes the call to `getParent().layout()`?
> 
> Edit: I also got another interesting NPE in `PrismTextLayout`:
> 
> java.lang.NullPointerException: Cannot read the array length because "this.runs" is null
>     at com.sun.javafx.text.PrismTextLayout.addTextRun(PrismTextLayout.java:770)
>     at com.sun.javafx.text.GlyphLayout.addTextRun(GlyphLayout.java:140)
>     at com.sun.javafx.text.GlyphLayout.breakRuns(GlyphLayout.java:210)
>     at com.sun.javafx.text.PrismTextLayout.buildRuns(PrismTextLayout.java:785)
>     at com.sun.javafx.text.PrismTextLayout.layout(PrismTextLayout.java:1036)
>     at com.sun.javafx.text.PrismTextLayout.ensureLayout(PrismTextLayout.java:223)
>     at com.sun.javafx.text.PrismTextLayout.getBounds(PrismTextLayout.java:246)
>     at javafx.scene.text.Text.getLogicalBounds(Text.java:432)
>     at javafx.scene.text.Text.doComputeGeomBounds(Text.java:1187)
>     at javafx.scene.text.Text$1.doComputeGeomBounds(Text.java:149)
>     at com.sun.javafx.scene.shape.TextHelper.computeGeomBoundsImpl(TextHelper.java:90)
>     at com.sun.javafx.scene.NodeHelper.computeGeomBounds(NodeHelper.java:116)
>     at javafx.scene.Node.updateGeomBounds(Node.java:3818)
>     at javafx.scene.Node.getGeomBounds(Node.java:3780)
>     at javafx.scene.Node.updateBounds(Node.java:776)
>     at javafx.scene.Parent.updateBounds(Parent.java:1833)
>     ...
>    <<pulse>>

> The simple null check helps in this case. But sometimes I see the exception, posted by @Maran23 . So the root cause is probably not fixed - but the symptoms are. I guess, this PR is then a good progress.

Maybe it's not good progress. You've investigated the issue, and have identified a likely root cause. Not fixing the root cause just means that it most likely won't be fixed for years to come, and the insights you've accumulated will fade.

I think we should just fix the root cause. If some application breaks as a result of this, not much harm is done: we'd gain valuable insight into a real-world situation that depends on the current behavior, and can then decide whether to revert the fix, or provide a viable alternative solution (for users) that accomplishes the same goal.

-------------

PR: https://git.openjdk.org/jfx/pull/564