From c6c114e534f63390796e6814f94dad163c859825 Mon Sep 17 00:00:00 2001
From: Lyubomir Marinov <lyubomir.marinov@jitsi.org>
Date: Thu, 6 Dec 2012 22:00:06 +0000
Subject: [PATCH] Addresses a media- and UI-related freeze.

---
 .../protocol/CachingPushBufferStream.java     | 137 +++++++++++++-----
 1 file changed, 97 insertions(+), 40 deletions(-)

diff --git a/src/org/jitsi/impl/neomedia/protocol/CachingPushBufferStream.java b/src/org/jitsi/impl/neomedia/protocol/CachingPushBufferStream.java
index bc7d14eb..d8644e6e 100644
--- a/src/org/jitsi/impl/neomedia/protocol/CachingPushBufferStream.java
+++ b/src/org/jitsi/impl/neomedia/protocol/CachingPushBufferStream.java
@@ -252,7 +252,9 @@ public Object getControl(String controlType)
 
         if ((control == null)
                 && BufferControl.class.getName().equals(controlType))
+        {
             control = getBufferControl();
+        }
         return control;
     }
 
@@ -281,11 +283,13 @@ public Object[] getControls()
             boolean bufferControlExists = false;
 
             for (Object control : controls)
+            {
                 if (control instanceof BufferControl)
                 {
                     bufferControlExists = true;
                     break;
                 }
+            }
             if (!bufferControlExists)
             {
                 BufferControl bufferControl = getBufferControl();
@@ -378,11 +382,11 @@ public void read(Buffer buffer)
         {
             if (readException != null)
             {
-                IOException ex = new IOException();
+                IOException ioe = new IOException();
 
-                ex.initCause(readException);
+                ioe.initCause(readException);
                 readException = null;
-                throw ex;
+                throw ioe;
             }
 
             buffer.setLength(0);
@@ -553,30 +557,26 @@ private int read(Buffer input, Buffer output, int outputOffset)
      */
     public void setTransferHandler(BufferTransferHandler transferHandler)
     {
-        synchronized (cache)
-        {
-            BufferTransferHandler substituteTransferHandler
-                = (transferHandler == null)
-                    ? null
-                    : new StreamSubstituteBufferTransferHandler(
-                            transferHandler,
-                            stream,
-                            this)
+        BufferTransferHandler substituteTransferHandler
+            = (transferHandler == null)
+                ? null
+                : new StreamSubstituteBufferTransferHandler(
+                        transferHandler,
+                        stream,
+                        this)
+                    {
+                        @Override
+                        public void transferData(PushBufferStream stream)
                         {
-                            @Override
-                            public void transferData(PushBufferStream stream)
-                            {
-                                if (CachingPushBufferStream.this.stream
-                                        == stream)
-                                {
-                                    CachingPushBufferStream.this.transferData(
-                                            this);
-                                }
-
-                                super.transferData(stream);
-                            }
-                        };
+                            if (CachingPushBufferStream.this.stream == stream)
+                                CachingPushBufferStream.this.transferData(this);
+
+                            super.transferData(stream);
+                        }
+                    };
 
+        synchronized (cache)
+        {
             stream.setTransferHandler(substituteTransferHandler);
             this.transferHandler = substituteTransferHandler;
             cache.notifyAll();
@@ -595,41 +595,98 @@ public void transferData(PushBufferStream stream)
      */
     protected void transferData(BufferTransferHandler transferHandler)
     {
+        /*
+         * Obviously, we cannot cache every Buffer because we will run out of
+         * memory. So wait for root to appear within cache (or for this instance
+         * to be stopped, of course).
+         */
+        boolean interrupted = false;
+        boolean canWriteInCache = false;
+
         synchronized (cache)
         {
-            boolean interrupted = false;
-
             while (true)
             {
                 if (this.transferHandler != transferHandler)
-                    return;
-                if (canWriteInCache())
+                {
+                    /*
+                     * The specified transferHandler has already been
+                     * obsoleted/replaced so it does not have the right to cause
+                     * a read or a write.
+                     */
+                    canWriteInCache = false;
                     break;
-                try
+                }
+                else if (canWriteInCache())
                 {
-                    cache.wait();
+                    canWriteInCache = true;
+                    break;
                 }
-                catch (InterruptedException iex)
+                else
                 {
-                    interrupted = true;
+                    try
+                    {
+                        cache.wait(DEFAULT_BUFFER_LENGTH);
+                    }
+                    catch (InterruptedException iex)
+                    {
+                        interrupted = true;
+                    }
                 }
             }
-            if (interrupted)
-                Thread.currentThread().interrupt();
-
+        }
+        if (interrupted)
+        {
+            Thread.currentThread().interrupt();
+        }
+        else if (canWriteInCache)
+        {
+            /*
+             * The protocol of PushBufferStream's #read(Buffer) method is that
+             * it does not block. The underlying implementation may be flawed
+             * though so we would better not take any chances. Besides, we have
+             * a report at the time of this writing which suggests that we may
+             * really be hitting a rogue implementation in a real-world
+             * scenario.
+             */
             Buffer buffer = new Buffer();
+            IOException readException;
 
             try
             {
                 stream.read(buffer);
                 readException = null;
             }
-            catch (IOException ioex)
+            catch (IOException ioe)
+            {
+                readException = ioe;
+            }
+            if (readException == null)
             {
-                readException = ioex;
+                if (!buffer.isDiscard()
+                        && (buffer.getLength() != 0)
+                        && (buffer.getData() != null))
+                {
+                    /*
+                     * Well, we risk disagreeing with #canWriteInCache() because
+                     * we have temporarily released the cache but we have read a
+                     * Buffer from the stream so it is probably better to not
+                     * throw it away.
+                     */
+                    synchronized (cache)
+                    {
+                        cache.add(buffer);
+                        cacheLengthInMillis += getLengthInMillis(buffer);
+                    }
+                }
+            }
+            else
+            {
+                synchronized (cache)
+                {
+                    this.readException = readException;
+                }
             }
-            cache.add(buffer);
-            cacheLengthInMillis += getLengthInMillis(buffer);
         }
     }
 
-- 
GitLab