Hi John and others,
I don't think we are entirely on the same page, so here's the objective.
The Goal: To determine if the FX animation thread and a SINGLE other
thread can access Animation in a safe manner wrt play, stop, and resume.
Non Goal: Multi-threaded access of Animation play, stop, and resume is NOT
a goal.
Wrt play() and resume(), it is ALWAYS safe to call them on a background
thread because at this point the Animation isn't being processed by the FX
thread as the "Animation" isn't contained in the AbstractPrimaryTimer
receivers array.
Wrt stop() from what I can tell there are no shared method calls that need
synchronization (see audit below), however the following two boolean flags
should be marked as volatile in order to ensure that animation is cut
short if executing:
TimelineClipCore.abort
ClipEnvelope.abort
This is simple enough to add to my original proposal of replacing the
arrays in AbstractPrimaryTimer with CopyOnWriteArrayList which is thread
safe and replicates the intended behavior in a clear and concise way.
Here are the methods that are called when stop() is invoked:
Timeline.stop()
getStatus()
clipCore.abort()
isStopped()
clipEnvelope.abortCurrentPulse()
doStop()
timer.removePulseReceiver(pulseReceiver);
// After this point it doesn't matter
setStatus(Status.STOPPED)
doSetCurrentRate(0.0)
jumpTo(Duration.ZERO)
And for AbstractPrimaryTimer.timePulseImpl:
AbstractPrimaryTimer.timePulseImpl
Animation.PulseReceiver.timePulse
Animation.doTimePulse
clipEnvelope.timePulse
animation.doPlayTo
clipCore.playTo
visitKeyFrame
setTime
animation.setCurrentTicks
clipInterpolator.interpolate
animation.doJumpTo
sync(false)
setCurrentTicks
clipCore.jumpTo
timeline.getStatus()
clipInterpolator.interpolate
Regards
Jurgen
On Tue, 23 Jan 2024 01:52:42 +0200, John Hendrikx
<john.hendr...@gmail.com> wrote:
This is not a good idea, the NPE is a symptom of the problem, but there
can be many more subtler problems (and new ones may surface after
>fixing the NPE) that can affect other animations making use of the
shared structures involved. For one thing, absent a method of
>synchronization, Java doesn't even guarantee that fields (or arrays)
contain the same value for all threads. So your thread may modify some
>state, but the FX thread wouldn't even know about it (or only
partially), and may make assumptions about an incorrect state, and then
make >further changes based on incorrect assumptions.
A good example is that even a much simpler class like HashMap when
accessed by multiple threads can get into a really bad state. It will
>usually throw a "ConcurrentModificationException" (or perhaps a weird
NPE or IllegalStateException) to alert you that you're using it wrong
>-- but those are the **best** case scenarios... In the worst case
scenario, modifying a HashMap on multiple threads can result in an
infinite >loop -- this can happen when two modifications occur, both
updating the same bucket, and the threads end up changing things in such
a way >that the entries point to each other. Iterating it then (or
accessing the wrong key) will cause an infinite loop (not hypothetical,
I've had this >happen).
I'm afraid that even the synchronization option I proposed is not very
realistic due to the amount of work it will entail. We'd need to trace
all >code paths that a call to play or stop could get to, and all those
code paths would need to be made thread safe.
--John
