On Wed, 8 Feb 2023 19:07:51 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> two questions: > > 1. should the timeline object be set to null here? the reference will > get overwritten in seriesRemoved():414, there is probably no need to keep > this object in memory > Yes timeline can be made null here. Updated the code. > 2. is it possible that `seriesBeingRemovedIsAdded()` will be invoked > twice with a different `series` argument? In other words, are we going to > face a situation when a `timeline` from unrelated series will not get > `stop`'ped? > > > (these question apply to other charts as well) Since `seriesBeingRemovedIsAdded()` is called from the event handler which gets invoked when the series changes and further there is a check on `setToRemove` boolean, I believe the method will not be called with different `series` and cause unwanted `series` timeline to be stopped. > tests/system/src/test/java/test/javafx/scene/control/XYChartExceptionOnAddingRemovedSeriesTest.java > line 315: > >> 313: stage.show(); >> 314: >> 315: Thread.currentThread().setUncaughtExceptionHandler((t2, e) >> -> { > > I think it's better to set this handler in a pre-test method annotated with > @Before, and clean up in @After - see, for instance, how it's done in > BehaviorCleanupTest. Updated code to set the handler in function annotated with @BeforeClass and cleaned up in @AfterClass. ------------- PR: https://git.openjdk.org/jfx/pull/1015