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

[danielpeintner]

On Wed, 21 Sep 2022 13:27:46 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

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

@mstr2 I see you arguments and your concerns. On the other hand, as a developer affected by this problem, it is different. Costumers are complaining and now it seems to occur even more frequently.

Note: I do not have any voting rights and I do not want to make any claims. Anyhow, I think we all agree that the simple null check *solves* the issue by hiding it and definitely does not create any regression. Hence, affected applications may get a quick fix and are running stable. The *actual* fix can come later, taking more time, with more sophisticated testing et cetera. I am happy to help there with our application as well. Thanks!

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

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