Addresses a possible deadlock in video calls.

2 files changed, 222 insertions, 69 deletions

diff --git a/src/net/java/sip/communicator/service/neomedia/event/VideoNotifierSupport.java b/src/net/java/sip/communicator/service/neomedia/event/VideoNotifierSupport.java
index 4dffb14..70be427 100644
--- a/src/net/java/sip/communicator/service/neomedia/event/VideoNotifierSupport.java
+++ b/src/net/java/sip/communicator/service/neomedia/event/VideoNotifierSupport.java
@@ -6,18 +6,27 @@
  */
 package net.java.sip.communicator.service.neomedia.event;
 
-import java.awt.Component;
+import java.awt.*;
 import java.util.*;
+import java.util.List; // disambiguation
 
 /**
  * Represents a mechanism to easily add to a specific <tt>Object</tt> by means
  * of composition support for firing <tt>VideoEvent</tt>s to
  * <tt>VideoListener</tt>s.
  *
- * @author Lubomir Marinov
+ * @author Lyubomir Marinov
  */
 public class VideoNotifierSupport
 {
+    private static final long THREAD_TIMEOUT = 5000;
+
+    /**
+     * The list of <tt>VideoEvent</tt>s which are to be delivered to the
+     * {@link #listeners} registered with this instance when
+     * {@link #synchronous} is equal to <tt>false</tt>.
+     */
+    private final List<VideoEvent> events;
 
     /**
      * The list of <tt>VideoListener</tt>s interested in changes in the
@@ -33,6 +42,18 @@ public class VideoNotifierSupport
     private final Object source;
 
     /**
+     * The indicator which determines whether this instance delivers the
+     * <tt>VideoEvent</tt>s to the {@link #listeners} synchronously.
+     */
+    private final boolean synchronous;
+
+    /**
+     * The <tt>Thread</tt> in which {@link #events} are delivered to the
+     * {@link #listeners} when {@link #synchronous} is equal to <tt>false</tt>.
+     */
+    private Thread thread;
+
+    /**
      * Initializes a new <tt>VideoNotifierSupport</tt> instance which is to
      * facilitate the management of <tt>VideoListener</tt>s and firing
      * <tt>VideoEvent</tt>s to them for a specific <tt>Object</tt>.
@@ -42,7 +63,25 @@ public class VideoNotifierSupport
      */
     public VideoNotifierSupport(Object source)
     {
+        this(source, true);
+    }
+
+    /**
+     * Initializes a new <tt>VideoNotifierSupport</tt> instance which is to
+     * facilitate the management of <tt>VideoListener</tt>s and firing
+     * <tt>VideoEvent</tt>s to them for a specific <tt>Object</tt>.
+     *
+     * @param source the <tt>Object</tt> which is to be reported as the source
+     * of the <tt>VideoEvent</tt>s fired by the new instance
+     * @param synchronous <tt>true</tt> if the new instance is to deliver the
+     * <tt>VideoEvent</tt>s synchronously; otherwise, <tt>false</tt>
+     */
+    public VideoNotifierSupport(Object source, boolean synchronous)
+    {
         this.source = source;
+        this.synchronous = synchronous;
+
+        events = this.synchronous ? null : new LinkedList<VideoEvent>();
     }
 
     /**
@@ -71,6 +110,32 @@ public class VideoNotifierSupport
         }
     }
 
+    protected void doFireVideoEvent(VideoEvent event)
+    {
+        VideoListener[] listeners;
+
+        synchronized (this.listeners)
+        {
+            listeners
+                = this.listeners.toArray(
+                        new VideoListener[this.listeners.size()]);
+        }
+
+        for (VideoListener listener : listeners)
+            switch (event.getType())
+            {
+                case VideoEvent.VIDEO_ADDED:
+                    listener.videoAdded(event);
+                    break;
+                case VideoEvent.VIDEO_REMOVED:
+                    listener.videoRemoved(event);
+                    break;
+                default:
+                    listener.videoUpdate(event);
+                    break;
+            }
+    }
+
     /**
      * Notifies the <tt>VideoListener</tt>s registered with this
      * <tt>VideoMediaStream</tt> about a specific type of change in the
@@ -83,6 +148,9 @@ public class VideoNotifierSupport
      * @param origin {@link VideoEvent#LOCAL} if the origin of the video is
      * local (e.g. it is being locally captured); {@link VideoEvent#REMOTE} if
      * the origin of the video is remote (e.g. a remote peer is streaming it)
+     * @param wait <tt>true</tt> if the call is to wait till the specified
+     * <tt>VideoEvent</tt> has been delivered to the <tt>VideoListener</tt>s;
+     * otherwise, <tt>false</tt>
      * @return <tt>true</tt> if this event and, more specifically, the visual
      * <tt>Component</tt> it describes have been consumed and should be
      * considered owned, referenced (which is important because
@@ -90,44 +158,14 @@ public class VideoNotifierSupport
      * otherwise, <tt>false</tt>
      */
     public boolean fireVideoEvent(
-            int type,
-            Component visualComponent,
-            int origin)
+            int type, Component visualComponent, int origin,
+            boolean wait)
     {
-        VideoListener[] listeners;
-
-        synchronized (this.listeners)
-        {
-            listeners
-                = this.listeners
-                    .toArray(new VideoListener[this.listeners.size()]);
-        }
-
-        boolean consumed;
-
-        if (listeners.length > 0)
-        {
-            VideoEvent event
-                = new VideoEvent(source, type, visualComponent, origin);
-
-            for (VideoListener listener : listeners)
-                switch (type)
-                {
-                    case VideoEvent.VIDEO_ADDED:
-                        listener.videoAdded(event);
-                        break;
-                    case VideoEvent.VIDEO_REMOVED:
-                        listener.videoRemoved(event);
-                        break;
-                    default:
-                        throw new IllegalArgumentException("type");
-                }
+        VideoEvent event
+            = new VideoEvent(source, type, visualComponent, origin);
 
-            consumed = event.isConsumed();
-        }
-        else
-            consumed = false;
-        return consumed;
+        fireVideoEvent(event, wait);
+        return event.isConsumed();
     }
 
     /**
@@ -136,31 +174,45 @@ public class VideoNotifierSupport
      *
      * @param event the <tt>VideoEvent</tt> to be fired to the
      * <tt>VideoListener</tt>s registered with this instance
+     * @param wait <tt>true</tt> if the call is to wait till the specified
+     * <tt>VideoEvent</tt> has been delivered to the <tt>VideoListener</tt>s;
+     * otherwise, <tt>false</tt>
      */
-    public void fireVideoEvent(VideoEvent event)
+    public void fireVideoEvent(VideoEvent event, boolean wait)
     {
-        VideoListener[] listeners;
-
-        synchronized (this.listeners)
+        if (synchronous)
+            doFireVideoEvent(event);
+        else
         {
-            listeners
-                = this.listeners
-                    .toArray(new VideoListener[this.listeners.size()]);
-        }
-
-        for (VideoListener listener : listeners)
-            switch (event.getType())
+            synchronized (events)
             {
-                case VideoEvent.VIDEO_ADDED:
-                    listener.videoAdded(event);
-                    break;
-                case VideoEvent.VIDEO_REMOVED:
-                    listener.videoRemoved(event);
-                    break;
-                default:
-                    listener.videoUpdate(event);
-                    break;
+                events.add(event);
+
+                if (thread == null)
+                    startThread();
+                else
+                    events.notify();
+
+                if (wait)
+                {
+                    boolean interrupted = false;
+
+                    while (events.contains(event) && (thread != null))
+                    {
+                        try
+                        {
+                            events.wait();
+                        }
+                        catch (InterruptedException ie)
+                        {
+                            interrupted = true;
+                        }
+                    }
+                    if (interrupted)
+                        Thread.currentThread().interrupt();
+                }
             }
+        }
     }
 
     /**
@@ -179,4 +231,97 @@ public class VideoNotifierSupport
             listeners.remove(listener);
         }
     }
+
+    private void runInThread()
+    {
+        while (true)
+        {
+            VideoEvent event = null;
+
+            synchronized (events)
+            {
+                long emptyTime = -1;
+                boolean interrupted = false;
+
+                while (events.isEmpty())
+                {
+                    if (emptyTime == -1)
+                        emptyTime = System.currentTimeMillis();
+                    else
+                    {
+                        long newEmptyTime = System.currentTimeMillis();
+
+                        if ((newEmptyTime - emptyTime) >= THREAD_TIMEOUT)
+                        {
+                            events.notify();
+                            return;
+                        }
+                    }
+
+                    try
+                    {
+                        events.wait(THREAD_TIMEOUT);
+                    }
+                    catch (InterruptedException ie)
+                    {
+                        interrupted = true;
+                    }
+                }
+                if (interrupted)
+                    Thread.currentThread().interrupt();
+
+                event = events.remove(0);
+            }
+
+            if (event != null)
+            {
+                try
+                {
+                    doFireVideoEvent(event);
+                }
+                catch (Throwable t)
+                {
+                    if (t instanceof ThreadDeath)
+                        throw (ThreadDeath) t;
+                }
+
+                synchronized (events)
+                {
+                    events.notify();
+                }
+            }
+        }
+    }
+
+    private void startThread()
+    {
+        thread
+            = new Thread("VideoNotifierSupportThread")
+            {
+                @Override
+                public void run()
+                {
+                    try
+                    {
+                        runInThread();
+                    }
+                    finally
+                    {
+                        synchronized (events)
+                        {
+                            if (Thread.currentThread().equals(thread))
+                            {
+                                thread = null;
+                                if (events.isEmpty())
+                                    events.notify();
+                                else
+                                    startThread();
+                            }
+                        }
+                    }
+                }
+            };
+        thread.setDaemon(true);
+        thread.start();
+    }
 }
diff --git a/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java b/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java
index e71e0b6..efcf238 100644
--- a/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java
+++ b/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java
@@ -375,12 +375,14 @@ public abstract class MediaAwareCallPeer
     public void setLocalVideoAllowed(boolean allowed)
         throws OperationFailedException
     {
-        if(getMediaHandler().isLocalVideoTransmissionEnabled() == allowed)
-            return;
+        CallPeerMediaHandler<?> mediaHandler = getMediaHandler();
 
-        // Modify the local media setup to reflect the requested setting for
-        // the streaming of the local video.
-        getMediaHandler().setLocalVideoTransmissionEnabled(allowed);
+        if(mediaHandler.isLocalVideoTransmissionEnabled() != allowed)
+        {
+            // Modify the local media setup to reflect the requested setting for
+            // the streaming of the local video.
+            mediaHandler.setLocalVideoTransmissionEnabled(allowed);
+        }
     }
 
     /**
@@ -509,14 +511,16 @@ public abstract class MediaAwareCallPeer
         // of CallPeerMediaHandler) we won't set and fire the current state
         // to Disconnected. Before closing the mediaHandler is setting the state
         // in order to deliver states as quick as possible.
-        synchronized(getMediaHandler())
+        CallPeerMediaHandler<?> mediaHandler = getMediaHandler();
+
+        synchronized(mediaHandler)
         {
             super.setState(newState, reason, reasonCode);
 
             if (CallPeerState.DISCONNECTED.equals(newState)
                     || CallPeerState.FAILED.equals(newState))
             {
-                getMediaHandler().close();
+                mediaHandler.close();
             }
         }
     }
@@ -788,8 +792,10 @@ public abstract class MediaAwareCallPeer
             // us audio for at least two separate participants. We therefore
             // need to remove the stream level listeners and switch to CSRC
             // level listening
-            getMediaHandler().setStreamAudioLevelListener(null);
-            getMediaHandler().setCsrcAudioLevelListener(this);
+            CallPeerMediaHandler<?> mediaHandler = getMediaHandler();
+
+            mediaHandler.setStreamAudioLevelListener(null);
+            mediaHandler.setCsrcAudioLevelListener(this);
         }
     }
 
@@ -811,8 +817,10 @@ public abstract class MediaAwareCallPeer
             // since there's only us and her in the call. Lets stop being a CSRC
             // listener and move back to listening the audio level of the
             // stream itself.
-            getMediaHandler().setStreamAudioLevelListener(this);
-            getMediaHandler().setCsrcAudioLevelListener(null);
+            CallPeerMediaHandler<?> mediaHandler = getMediaHandler();
+
+            mediaHandler.setStreamAudioLevelListener(this);
+            mediaHandler.setCsrcAudioLevelListener(null);
         }
     }