Bug ID: JDK-8123403 Threading issues with ProgressBar / ProgressIndicator after fix for RT-27791

Versions (Unresolved/Resolved/Fixed)

The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.

JDK 8
8Fixed

The recent change to fix RT-27791 has introduced two threading issues that must be addressed.

changeset:   4570:3aadad7d5904
user:        mickf
date:        Thu Aug 01 14:43:17 2013 +0100
summary:     RT-27791 :ProgressBar keeps animating when in ScrollPane and scrolled out of view

First, the threads are not created as daemon threads, which can lead to applications that will not exit. To see this problem, run the Modena app and close the window (using the close button on the window frame). The app will continue to run. See the attached thread stack dump.

Second, the code is not MT safe (this is from inspection), which could lead to other issues. It calls timeline.play() from the progress indicator thread, which should only be done on the FX application thread.

Can you comment on why a separate thread is needed in the first place? If you are able to use an animation timeline with a key frame that wakes up every N milliseconds, it seems that both problems would be solved. If this somehow isn't possible then two changes need to be made:

1) Call Thread.setDaemon(true) before starting the thread

2) Wrap the call to the Timeline.play() methods in Platform.runLater()

Verified on 8.0b114 and code version of RT repo of 5642
06-11-2013
The thread has been removed, and RT-32627 has been filed for the 'extends' issue.
30-08-2013
I've created RT-32627 to track the issue that ProgressBarSkin doesn't extend ProgressIndicatorSkin
30-08-2013
The thread problems should be solved with : changeset: 4847:0339d4004e85 tag: tip user: mickf date: Fri Aug 30 15:41:07 2013 +0100 summary: RT-32327 : Threading issues with ProgressBar / ProgressIndicator after fix for RT-27791
30-08-2013
I'll push the fix today, and file a separate jira for the re-factoring.
30-08-2013
Hi Mick, do we have an ETA on this fix?
30-08-2013
A few other comments. In this patch, we see the same code copy & pasted in two places. Since ProgressBar extends ProgressIndicator, it seems like a better idea to either have ProgressBarSkin extend ProgressIndicatorSkin, or at the least have a ProgressSkinBase from which both extend where the common code can live. Second, the comment style should be fixed. We need to be consistent and this style: /* ** / although objectively just as useful as the normal one employed by the Java code convention guidelines, should be avoided in favor of the standard javadoc comment syntax: /* * / for methods and classes, or // for comments within a method. Sometimes we use / * */ within a method but very rarely and I tend to avoid it. Also, I don't understand the need for another thread in this code? If you want to throttle it, you can always do so using a counter and mod rather than an entire separate thread per ProgressBar or ProgressIndicator?
21-08-2013
I am seeing the MT issues show up with Modena. See RT-32443.
20-08-2013
Another option might be to use a javafx.concurrent.Task which will help manage the tasks without requiring a separate thread for each progress bar / progress indicator, which is another drawback of the current approach.
14-08-2013

Relates :	JDK-8124471 - ProgressBar keeps animating when in ScrollPane and scrolled out of view
Relates :	JDK-8123806 - StackOverflowError in Modena
Relates :	JDK-8123094 - Apps with ProgressBar don't exit any more because of Regression on RT-32327
Relates :	JDK-8097782 - ProgressBarSkin should extend ProgressIndicatorSkin.