Insubstantial / insubstantial

Swing look-and-feel library and assorted widgets
193 stars 58 forks source link

EDT or Trident callback thread freezes #145

Open Sebast91 opened 8 years ago

Sebast91 commented 8 years ago

Freeze of EDT or Trident callback thread takes place in StateTransitionMultiTracker due to concurrent access on {@link StateTransitionMultiTracker#trackerMap} in implementation of methods {@link StateTransitionListener#onModelStateTransition(StateTransitionEvent)} and {@link StateTransitionListener#onFocusStateTransition(StateTransitionEvent)} in {@link StateTransitionMultiTracker#addTracker(Comparable, StateTransitionTracker)} method.

In StateTransitionTracker all events are not delegated to EDT:

        // notify listeners on focus state transition
        this.focusTimeline.addCallback(new TimelineCallbackAdapter() {
            @Override
            public void onTimelineStateChanged(TimelineState oldState,
                    TimelineState newState, float durationFraction,
                    float timelinePosition) {
                fireFocusStateTransitionEvent(oldState, newState);
            }
        });

How to fix: delegate the call to {@link StateTransitionTracker#fireFocusStateTransitionEvent(TimelineState, TimelineState)} in EDT:

        // notify listeners on focus state transition
        this.focusTimeline.addCallback(new TimelineCallbackAdapter() {
            @Override
            public void onTimelineStateChanged(TimelineState oldState,
                    TimelineState newState, float durationFraction,
                    float timelinePosition) {
                SwingUtilities.invokeLater(new Runnable() {
                    @Override
                    public void run() {
                        fireFocusStateTransitionEvent(oldState, newState);
                    }
            }
        });

Regards, Sébastien Gollion.

IvanRF commented 7 years ago

For easier access, this is the line: StateTransitionTracker.java#L216

Which was the symptom that lead you to this issue? I mean, what was happening to your app when you found this? I ask because I'm dealing with a random app freeze and I can't find the reason.

spslinger commented 7 years ago

I agree that the trackermap in StateTransitionMultiTracker is causing freezes, but I don't agree with the solution. The problem is incomplete synchronization of the trackermap in StateTransitionMultiTracker itself.

Most methods of StateTransitionMultiTracker are synchronized, protecting the map from concurrent access... except for the trackermap.remove calls in the StateTransitionListener. Adding a synchronized removeTracker method and calling that from the StateTransitionListener in addTracker would guarantee thread-safety of the trackermap within StateTransitionMultiTracker itself and not rely on "calling it on the right thread" from other classes. Who says other classes will not call the getTracker, clear or addTracker methods from a non-EDT thread while you're calling trackermap.remove on the EDT, and in doing so still corrupt the HashMap, causing your code to loop infinitely on a HashMap.get?