On Fri, 20 Sep 2024 06:08:52 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/QuantumToolkit.java
>> line 276:
>>
>>> 274: try {
>>> 275: if (!disposeLatch.await(5, TimeUnit.SECONDS)) {
>>> 276: throw new InternalError("dispose timed out");
>>
>> could this happen on a real system? if so, should the message be more clear
>> and user-friendly?
>>
>> also, if it can be encountered in the field, it might be better to declare a
>> constant and add a javadoc to it explaining what it is.
>>
>> what do you think?
>
> I think javadoc and a property might be an overkill at this moment. The root
> problem is the deadlock and is very intermittent, and it hinders with the
> jenkins jobs. The change seems good means to avoid such situations where
> process keep running unchecked for hours. And if in case the frequency of
> issue increases or we could find a definite way of reproducing the issue,
> then we should try to fix the deadlock.
> Approving, I shall re-approve if updated.
I also think that's overkill to document this or make it programmable. This
timed wait is to handle the (very rare) case of a hang/deadlock that will
otherwise prevent the application from exiting at all. Remember that this code
only gets run when the application calls `System.exit(int)`.
I could provide a slightly more informative message. Maybe something like:
Timeout shutting down the JavaFX toolkit
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1574#discussion_r1768482904
