DataPointTooltip missing dataset synchronization

[dedeibel]

I think there might be race condition in the DataPointTooltip code regarding the lookup of points near the mouse and getting the data set's values.

First the index of valid points is fetched, then later it's value is fetched. In the meantime the dataset could have been cleared resulting in an out of bounds exception.

https://github.com/GSI-CS-CO/chart-fx/blob/fc81f1fa2fced4d4102bbc4fa3558cef35742a7e/chartfx-chart/src/main/java/de/gsi/chart/plugins/DataPointTooltip.java#L126

And then:

https://github.com/GSI-CS-CO/chart-fx/blob/fc81f1fa2fced4d4102bbc4fa3558cef35742a7e/chartfx-chart/src/main/java/de/gsi/chart/plugins/DataPointTooltip.java#L134

I observed the following stacktrace regarding this:

StackTrace
    java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0
    	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
    	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
    	at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248)
    	at java.base/java.util.Objects.checkIndex(Objects.java:372)
    	at java.base/java.util.ArrayList.get(ArrayList.java:459)
    	at de.gsi.dataset.spi.LabelledMarkerDataSet.get(LabelledMarkerDataSet.java:74)
    	at de.gsi.chart.plugins.DataPointTooltip.getDataPointFromDataSet(DataPointTooltip.java:134)
    	at de.gsi.chart.plugins.DataPointTooltip.lambda$getPointsCloseToCursor$5(DataPointTooltip.java:129)
    	at java.base/java.util.stream.IntPipeline$1$1.accept(IntPipeline.java:180)
    	at java.base/java.util.stream.Streams$RangeIntSpliterator.forEachRemaining(Streams.java:104)
    	at java.base/java.util.Spliterator$OfInt.forEachRemaining(Spliterator.java:699)
    	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
    	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
    	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
    	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:274)
    	at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(AbstractList.java:720)
    	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:658)
    	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:274)
    	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
    	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
    	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
    	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
    	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:274)
    	at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(AbstractList.java:720)
    	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
    	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    	at java.base/java.util.stream.ReferencePipeline.reduce(ReferencePipeline.java:558)
    	at de.gsi.chart.plugins.DataPointTooltip.findNearestDataPointWithinPickingDistance(DataPointTooltip.java:109)
    	at de.gsi.chart.plugins.DataPointTooltip.findDataPoint(DataPointTooltip.java:97)
    	at de.gsi.chart.plugins.DataPointTooltip.updateToolTip(DataPointTooltip.java:235)
    	at javafx.base/com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:218)
    	at javafx.base/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
    	at javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:238)
    	at javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191)
    	at javafx.base/com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    	at javafx.base/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
    	at javafx.base/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
    	at javafx.base/javafx.event.Event.fireEvent(Event.java:198)
    	at javafx.graphics/javafx.scene.Scene$MouseHandler.process(Scene.java:3862)
    	at javafx.graphics/javafx.scene.Scene.processMouseEvent(Scene.java:1849)
    	at javafx.graphics/javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2590)
    	at javafx.graphics/com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:409)
    	at javafx.graphics/com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:299)
    	at java.base/java.security.AccessController.doPrivileged(Native Method)
    	at javafx.graphics/com.sun.javafx.tk.quantum.GlassViewEventHandler.lambda$handleMouseEvent$2(GlassViewEventHandler.java:447)
    	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.runWithoutRenderLock(QuantumToolkit.java:412)
    	at javafx.graphics/com.sun.javafx.tk.quantum.GlassViewEventHandler.handleMouseEvent(GlassViewEventHandler.java:446)
    	at javafx.graphics/com.sun.glass.ui.View.handleMouseEvent(View.java:556)
    	at javafx.graphics/com.sun.glass.ui.View.notifyMouse(View.java:942)
    	at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
    	at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(GtkApplication.java:277)
    	at java.base/java.lang.Thread.run(Thread.java:834)

Happened 2020-11-30 13:37:04 +01:00.

Do you have a suggestion on how to fix this? I think a mechanism to search the dataset using a lambda could be a nice and versatile solution. Additionally the DS could decide for itself if it is sorted or not.

Like (just a sketch):

List<DataPoint> closePoints = d.findDataPoints<DataPoint>(DataSet.DIM_X, idx -> { // iterate along X axis with surrounding read lock
            DataPoint point = new DataPoint(renderer, d.get(DataSet.DIM_X, i), d.get(DataSet.DIM_Y, i), getDataLabelSafe(d, i));
            double x = xAxis.getDisplayPosition(point.x);
            double y = yAxis.getDisplayPosition(point.y);
            Point2D displayPoint = new Point2D(x, y);
            point.distanceFromMouse = displayPoint.distance(mouseLocation);
            if (point.distanceFromMouse > getPickingDistance()) {
                return null; // null values are not added to result list
            }
            return point;
        });

Or maybe the plugin could simply issue a lock and unlock.

[dedeibel]

It seems there is already the readLockGuard with a similar concept.

[wirew0rm]

Yes readLockGuard would probably be the preferred way to make this safe. One problem I see is the way the code is written now it would need to block all DataSets for the whole search. This should probably be changed to extract the relevant information from each DataSet inside of a read lock guard and then decide afterwards which one is the closest and should be shown. Otherwise the next problem will occur when it tries to get the label for a point which was deleted in the meantime.
I rewrote a lot of this code recently to take situations with multiple renderers and datasets into account, I could have probably fixed that in the same go.

Thanks for reporting and the detailed description 👍

re dataset.isSorted(), we discussed this a lot already, but there along with the benefits there are some complications that prevented us from implementing this. Main points being: all code has to offer paths for both sorted and unsorted, how to ensure sortedness on dataset manipulation, what happens with multidimensional datasets, more to implement for people using their own datasets.

[wirew0rm]

fixed by #339