Phonon on windows: fixeda possible dead-lock when using QFile

the msdn was not correct and IAsyncReader::WaitForNext should not
return an error when the pin is currently flushing.
I took the opportunity to clean the way the mutex is locked on the
pin.

Task-number: 258830

9 files changed, 90 insertions, 152 deletions

diff --git a/src/3rdparty/phonon/ds9/iodevicereader.cpp b/src/3rdparty/phonon/ds9/iodevicereader.cpp
index a885a69..e0c505c 100644
--- a/src/3rdparty/phonon/ds9/iodevicereader.cpp
+++ b/src/3rdparty/phonon/ds9/iodevicereader.cpp
@@ -63,7 +63,6 @@ namespace Phonon
               //for Phonon::StreamInterface
               void writeData(const QByteArray &data)
               {
-                  QWriteLocker locker(&m_lock);
                   m_pos += data.size();
                   m_buffer += data;
               }
@@ -74,54 +73,22 @@ namespace Phonon
 
               void setStreamSize(qint64 newSize)
               {
-                  QWriteLocker locker(&m_lock);
+                  QMutexLocker locker(&m_mutex);
                   m_size = newSize;
               }
 
-              qint64 streamSize() const
-              {
-                  QReadLocker locker(&m_lock);
-                  return m_size;
-              }
-
               void setStreamSeekable(bool s)
               {
-                  QWriteLocker locker(&m_lock);
+                  QMutexLocker locker(&m_mutex);
                   m_seekable = s;
               }
 
-              bool streamSeekable() const
-              {
-                  QReadLocker locker(&m_lock);
-                  return m_seekable;
-              }
-
-              void setCurrentPos(qint64 pos)
-              {
-                  QWriteLocker locker(&m_lock);
-                  m_pos = pos;
-                  seekStream(pos);
-                  m_buffer.clear();
-              }
-
-              qint64 currentPos() const
-              {
-                  QReadLocker locker(&m_lock);
-                  return m_pos;
-              }
-
-              int currentBufferSize() const
-              {
-                  QReadLocker locker(&m_lock);
-                  return m_buffer.size();
-              }
-
               //virtual pure members
 
               //implementation from IAsyncReader
               STDMETHODIMP Length(LONGLONG *total, LONGLONG *available)
               {
-                  QReadLocker locker(&m_lock);
+                  QMutexLocker locker(&m_mutex);
                   if (total) {
                       *total = m_size;
                   }
@@ -136,44 +103,42 @@ namespace Phonon
 
               HRESULT read(LONGLONG pos, LONG length, BYTE *buffer, LONG *actual)
               {
-                  QMutexLocker locker(&m_mutexRead);
-
+                  Q_ASSERT(!m_mutex.tryLock());
                   if (m_mediaGraph->isStopping()) {
                       return VFW_E_WRONG_STATE;
                   }
 
-                  if(streamSize() != 1 && pos + length > streamSize()) {
+                  if(m_size != 1 && pos + length > m_size) {
                       //it tries to read outside of the boundaries
                       return E_FAIL;
                   }
 
-                  if (currentPos() - currentBufferSize() != pos) {
-                      if (!streamSeekable()) {
+                  if (m_pos - m_buffer.size() != pos) {
+                      if (!m_seekable) {
                           return S_FALSE;
                       }
-                      setCurrentPos(pos);
+                      m_pos = pos;
+                      seekStream(pos);
+                      m_buffer.clear();
                   }
 
-                  int oldSize = currentBufferSize();
-                  while (currentBufferSize() < int(length)) {
+                  int oldSize = m_buffer.size();
+                  while (m_buffer.size() < int(length)) {
                       needData();
                       if (m_mediaGraph->isStopping()) {
                           return VFW_E_WRONG_STATE;
                       }
 
-                      if (oldSize == currentBufferSize()) {
+                      if (oldSize == m_buffer.size()) {
                           break; //we didn't get any data
                       }
-                      oldSize = currentBufferSize();
+                      oldSize = m_buffer.size();
                   }
 
-                  int bytesRead = qMin(currentBufferSize(), int(length));
-                  {
-                      QWriteLocker locker(&m_lock);
-                      qMemCopy(buffer, m_buffer.data(), bytesRead);
-                      //truncate the buffer
-                      m_buffer = m_buffer.mid(bytesRead);
-                  }
+                  int bytesRead = qMin(m_buffer.size(), int(length));
+                  qMemCopy(buffer, m_buffer.data(), bytesRead);
+                  //truncate the buffer
+                  m_buffer = m_buffer.mid(bytesRead);
 
                   if (actual) {
                       *actual = bytesRead; //initialization
@@ -189,7 +154,6 @@ namespace Phonon
             qint64 m_pos;
             qint64 m_size;
 
-            QMutex m_mutexRead;
             const MediaGraph *m_mediaGraph;
         };
 
@@ -203,14 +167,6 @@ namespace Phonon
         IODeviceReader::~IODeviceReader()
         {
         }
-
-        STDMETHODIMP IODeviceReader::Stop()
-        {
-            HRESULT hr = QBaseFilter::Stop();
-            m_streamReader->enoughData(); //this asks to cancel any blocked call to needData
-            return hr;
-        }
-
     }
 }
 
diff --git a/src/3rdparty/phonon/ds9/iodevicereader.h b/src/3rdparty/phonon/ds9/iodevicereader.h
index af4b271..c8b91c3 100644
--- a/src/3rdparty/phonon/ds9/iodevicereader.h
+++ b/src/3rdparty/phonon/ds9/iodevicereader.h
@@ -41,7 +41,6 @@ namespace Phonon
         public:
             IODeviceReader(const MediaSource &source, const MediaGraph *);
             ~IODeviceReader();
-            STDMETHODIMP Stop();
 
         private:
             StreamReader *m_streamReader;
diff --git a/src/3rdparty/phonon/ds9/qasyncreader.cpp b/src/3rdparty/phonon/ds9/qasyncreader.cpp
index 68ec1f8..a3f9cda 100644
--- a/src/3rdparty/phonon/ds9/qasyncreader.cpp
+++ b/src/3rdparty/phonon/ds9/qasyncreader.cpp
@@ -15,8 +15,6 @@ You should have received a copy of the GNU Lesser General Public License
 along with this library.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-#include <QtCore/QFile>
-
 #include "qasyncreader.h"
 #include "qbasefilter.h"
 
@@ -80,8 +78,7 @@ namespace Phonon
 
         STDMETHODIMP QAsyncReader::Request(IMediaSample *sample,DWORD_PTR user)
         {
-            QMutexLocker mutexLocker(&m_mutexWait);
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (m_flushing) {
                 return VFW_E_WRONG_STATE;
             }
@@ -93,33 +90,28 @@ namespace Phonon
 
         STDMETHODIMP QAsyncReader::WaitForNext(DWORD timeout, IMediaSample **sample, DWORD_PTR *user)
         {
-            QMutexLocker locker(&m_mutexWait);
+            QMutexLocker locker(&m_mutex);
             if (!sample ||!user) {
                 return E_POINTER;
             }
 
+            //msdn says to return immediately if we're flushing but that doesn't seem to be true
+            //since it triggers a dead-lock somewhere inside directshow (see task 258830)
+
             *sample = 0;
             *user = 0;
 
-            AsyncRequest r = getNextRequest();
-
-            if (r.sample == 0) {
-                //there is no request in the queue
-                if (isFlushing()) {
+            if (m_requestQueue.isEmpty()) {
+                if (m_requestWait.wait(&m_mutex, timeout) == false) {
+                    return VFW_E_TIMEOUT;
+                }
+                if (m_requestQueue.isEmpty()) {
                     return VFW_E_WRONG_STATE;
-                } else {
-                    //First we need to lock the mutex
-                    if (m_requestWait.wait(&m_mutexWait, timeout) == false) {
-                        return VFW_E_TIMEOUT;
-                    }
-                    if (isFlushing()) {
-                        return VFW_E_WRONG_STATE;
-                    }
-
-                    r = getNextRequest();
                 }
             }
 
+            AsyncRequest r = m_requestQueue.dequeue();
+
             //at this point we're sure to have a request to proceed
             if (r.sample == 0) {
                 return E_FAIL;
@@ -127,14 +119,12 @@ namespace Phonon
 
             *sample = r.sample;
             *user = r.user;
-
-            return SyncReadAligned(r.sample);
+            return syncReadAlignedUnlocked(r.sample);
         }
 
         STDMETHODIMP QAsyncReader::BeginFlush()
         {
-            QMutexLocker mutexLocker(&m_mutexWait);
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_flushing = true;
             m_requestWait.wakeOne();
             return S_OK;
@@ -142,13 +132,28 @@ namespace Phonon
 
         STDMETHODIMP QAsyncReader::EndFlush()
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_flushing = false;
             return S_OK;
         }
 
         STDMETHODIMP QAsyncReader::SyncReadAligned(IMediaSample *sample)
         {
+            QMutexLocker locker(&m_mutex);
+            return syncReadAlignedUnlocked(sample);
+        }
+
+        STDMETHODIMP QAsyncReader::SyncRead(LONGLONG pos, LONG length, BYTE *buffer)
+        {
+            QMutexLocker locker(&m_mutex);
+            return read(pos, length, buffer, 0);
+        }
+
+
+        STDMETHODIMP QAsyncReader::syncReadAlignedUnlocked(IMediaSample *sample)
+        {
+            Q_ASSERT(!m_mutex.tryLock());
+
             if (!sample) {
                 return E_POINTER;
             }
@@ -175,23 +180,6 @@ namespace Phonon
             return sample->SetActualDataLength(actual);
         }
 
-        STDMETHODIMP QAsyncReader::SyncRead(LONGLONG pos, LONG length, BYTE *buffer)
-        {
-            return read(pos, length, buffer, 0);
-        }
-
-
-        //addition
-        QAsyncReader::AsyncRequest QAsyncReader::getNextRequest()
-        {
-            QWriteLocker locker(&m_lock);
-            AsyncRequest ret;
-            if (!m_requestQueue.isEmpty()) {
-                ret = m_requestQueue.dequeue();
-            }
-
-            return ret;
-        }
     }
 }
 
diff --git a/src/3rdparty/phonon/ds9/qasyncreader.h b/src/3rdparty/phonon/ds9/qasyncreader.h
index cb789ee..95872f9 100644
--- a/src/3rdparty/phonon/ds9/qasyncreader.h
+++ b/src/3rdparty/phonon/ds9/qasyncreader.h
@@ -48,11 +48,12 @@ namespace Phonon
             STDMETHODIMP WaitForNext(DWORD,IMediaSample **,DWORD_PTR *);
             STDMETHODIMP SyncReadAligned(IMediaSample *);
             STDMETHODIMP SyncRead(LONGLONG,LONG,BYTE *);
-            virtual STDMETHODIMP Length(LONGLONG *,LONGLONG *) = 0;
+            STDMETHODIMP Length(LONGLONG *,LONGLONG *) = 0;
             STDMETHODIMP BeginFlush();
             STDMETHODIMP EndFlush();
 
         protected:
+            STDMETHODIMP syncReadAlignedUnlocked(IMediaSample *);
             virtual HRESULT read(LONGLONG pos, LONG length, BYTE *buffer, LONG *actual) = 0;
 
         private:
@@ -62,9 +63,6 @@ namespace Phonon
                 IMediaSample *sample;
                 DWORD_PTR user;
             };
-            AsyncRequest getNextRequest();
-
-            QMutex m_mutexWait;
 
             QQueue<AsyncRequest> m_requestQueue;
             QWaitCondition m_requestWait;
diff --git a/src/3rdparty/phonon/ds9/qbasefilter.cpp b/src/3rdparty/phonon/ds9/qbasefilter.cpp
index c950c41..34b7660 100644
--- a/src/3rdparty/phonon/ds9/qbasefilter.cpp
+++ b/src/3rdparty/phonon/ds9/qbasefilter.cpp
@@ -166,19 +166,19 @@ namespace Phonon
 
         const QList<QPin *> QBaseFilter::pins() const
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             return m_pins;
         }
 
         void QBaseFilter::addPin(QPin *pin)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_pins.append(pin);
         }
 
         void QBaseFilter::removePin(QPin *pin)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_pins.removeAll(pin);
         }
 
@@ -251,35 +251,35 @@ namespace Phonon
 
         STDMETHODIMP QBaseFilter::GetClassID(CLSID *clsid)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             *clsid = m_clsid;
             return S_OK;
         }
 
         STDMETHODIMP QBaseFilter::Stop()
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_state = State_Stopped;
             return S_OK;
         }
 
         STDMETHODIMP QBaseFilter::Pause()
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_state = State_Paused;
             return S_OK;
         }
 
         STDMETHODIMP QBaseFilter::Run(REFERENCE_TIME)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_state = State_Running;
             return S_OK;
         }
 
         STDMETHODIMP QBaseFilter::GetState(DWORD, FILTER_STATE *state)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (!state) {
                 return E_POINTER;
             }
@@ -290,7 +290,7 @@ namespace Phonon
 
         STDMETHODIMP QBaseFilter::SetSyncSource(IReferenceClock *clock)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (clock) {
                 clock->AddRef();
             }
@@ -303,7 +303,7 @@ namespace Phonon
 
         STDMETHODIMP QBaseFilter::GetSyncSource(IReferenceClock **clock)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (!clock) {
                 return E_POINTER;
             }
@@ -342,7 +342,7 @@ namespace Phonon
 
         STDMETHODIMP QBaseFilter::QueryFilterInfo(FILTER_INFO *info )
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (!info) {
                 return E_POINTER;
             }
@@ -356,7 +356,7 @@ namespace Phonon
 
         STDMETHODIMP QBaseFilter::JoinFilterGraph(IFilterGraph *graph, LPCWSTR name)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_graph = graph;
             m_name = QString::fromUtf16((const unsigned short*)name);
             return S_OK;
diff --git a/src/3rdparty/phonon/ds9/qbasefilter.h b/src/3rdparty/phonon/ds9/qbasefilter.h
index 85f1431..a72d6fe 100644
--- a/src/3rdparty/phonon/ds9/qbasefilter.h
+++ b/src/3rdparty/phonon/ds9/qbasefilter.h
@@ -22,7 +22,7 @@ along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 #include <QtCore/QString>
 #include <QtCore/QList>
-#include <QtCore/QReadWriteLock>
+#include <QtCore/QMutex>
 
 #include <dshow.h>
 
@@ -127,7 +127,7 @@ namespace Phonon
             IFilterGraph *m_graph;
             FILTER_STATE m_state;
             QList<QPin *> m_pins;
-            mutable QReadWriteLock m_lock;
+            mutable QMutex m_mutex;
         };
     }
 }
diff --git a/src/3rdparty/phonon/ds9/qmeminputpin.cpp b/src/3rdparty/phonon/ds9/qmeminputpin.cpp
index 865b8af..630cfb4 100644
--- a/src/3rdparty/phonon/ds9/qmeminputpin.cpp
+++ b/src/3rdparty/phonon/ds9/qmeminputpin.cpp
@@ -84,7 +84,7 @@ namespace Phonon
                     conn->BeginFlush();
                 }
             }
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_flushing = true;
             return S_OK;
         }
@@ -98,7 +98,7 @@ namespace Phonon
                     conn->EndFlush();
                 }
             }
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_flushing = false;
             return S_OK;
         }
@@ -152,7 +152,7 @@ namespace Phonon
             }
 
             {
-                QWriteLocker locker(&m_lock);
+                QMutexLocker locker(&m_mutex);
                 m_shouldDuplicateSamples = m_transform && readonly;
             }
 
@@ -266,19 +266,19 @@ namespace Phonon
 
         void QMemInputPin::addOutput(QPin *output)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_outputs += output;
         }
 
         void QMemInputPin::removeOutput(QPin *output)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             m_outputs.removeOne(output);
         }
 
         QList<QPin*> QMemInputPin::outputs() const
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             return m_outputs;
         }
 
diff --git a/src/3rdparty/phonon/ds9/qpin.cpp b/src/3rdparty/phonon/ds9/qpin.cpp
index d14876b..55c952c 100644
--- a/src/3rdparty/phonon/ds9/qpin.cpp
+++ b/src/3rdparty/phonon/ds9/qpin.cpp
@@ -323,7 +323,7 @@ namespace Phonon
 
         STDMETHODIMP QPin::ConnectionMediaType(AM_MEDIA_TYPE *type)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (!type) {
                 return E_POINTER;
             }
@@ -338,7 +338,7 @@ namespace Phonon
 
         STDMETHODIMP QPin::QueryPinInfo(PIN_INFO *info)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (!info) {
                 return E_POINTER;
             }
@@ -346,14 +346,13 @@ namespace Phonon
             info->dir = m_direction;
             info->pFilter = m_parent;
             m_parent->AddRef();
-            qMemCopy(info->achName, m_name.utf16(), qMin(MAX_FILTER_NAME, m_name.length()+1) *2);
-
+            info->achName[0] = 0;
             return S_OK;
         }
 
         STDMETHODIMP QPin::QueryDirection(PIN_DIRECTION *dir)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (!dir) {
                 return E_POINTER;
             }
@@ -364,20 +363,19 @@ namespace Phonon
 
         STDMETHODIMP QPin::QueryId(LPWSTR *id)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (!id) {
                 return E_POINTER;
             }
 
-            int nbBytes = (m_name.length()+1)*2;
-            *id = static_cast<LPWSTR>(::CoTaskMemAlloc(nbBytes));
-            qMemCopy(*id, m_name.utf16(), nbBytes);
+            *id = static_cast<LPWSTR>(::CoTaskMemAlloc(2));
+            *id[0] = 0;
             return S_OK;
         }
 
         STDMETHODIMP QPin::QueryAccept(const AM_MEDIA_TYPE *type)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (!type) {
                 return E_POINTER;
             }
@@ -424,7 +422,7 @@ namespace Phonon
 
         STDMETHODIMP QPin::NewSegment(REFERENCE_TIME start, REFERENCE_TIME stop, double rate)
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (m_direction == PINDIR_OUTPUT && m_connected) {
                 //we deliver this downstream
                 m_connected->NewSegment(start, stop, rate);
@@ -505,7 +503,7 @@ namespace Phonon
 
         void QPin::setConnectedType(const AM_MEDIA_TYPE &type)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
 
             //1st we free memory
             freeMediaType(m_connectedType);
@@ -515,13 +513,13 @@ namespace Phonon
 
         const AM_MEDIA_TYPE &QPin::connectedType() const 
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             return m_connectedType;
         }
 
         void QPin::setConnected(IPin *pin)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (pin) {
                 pin->AddRef();
             }
@@ -533,7 +531,7 @@ namespace Phonon
 
         IPin *QPin::connected(bool addref) const
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (addref && m_connected) {
                 m_connected->AddRef();
             }
@@ -542,13 +540,13 @@ namespace Phonon
 
         bool QPin::isFlushing() const
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             return m_flushing;
         }
 
         FILTER_STATE QPin::filterState() const
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             FILTER_STATE fstate = State_Stopped;
             m_parent->GetState(0, &fstate);
             return fstate;
@@ -556,7 +554,7 @@ namespace Phonon
 
         QVector<AM_MEDIA_TYPE> QPin::mediaTypes() const
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             return m_mediaTypes;
         }
 
@@ -592,7 +590,7 @@ namespace Phonon
 
         void QPin::setMemoryAllocator(IMemAllocator *alloc)
         {
-            QWriteLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (alloc) {
                 alloc->AddRef();
             }
@@ -604,7 +602,7 @@ namespace Phonon
 
         IMemAllocator *QPin::memoryAllocator(bool addref) const
         {
-            QReadLocker locker(&m_lock);
+            QMutexLocker locker(&m_mutex);
             if (addref && m_memAlloc) {
                 m_memAlloc->AddRef();
             }
diff --git a/src/3rdparty/phonon/ds9/qpin.h b/src/3rdparty/phonon/ds9/qpin.h
index a3287c4..9af6077 100644
--- a/src/3rdparty/phonon/ds9/qpin.h
+++ b/src/3rdparty/phonon/ds9/qpin.h
@@ -22,7 +22,7 @@ along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 #include <QtCore/QString>
 #include <QtCore/QVector>
-#include <QtCore/QReadWriteLock>
+#include <QtCore/QMutex>
 
 #include <dshow.h>
 
@@ -85,7 +85,7 @@ namespace Phonon
 
         protected:
             //this can be used by sub-classes
-            mutable QReadWriteLock m_lock;
+            mutable QMutex m_mutex;
             QBaseFilter *m_parent;
             bool m_flushing;
 
@@ -98,7 +98,6 @@ namespace Phonon
             const PIN_DIRECTION m_direction;
             QVector<AM_MEDIA_TYPE> m_mediaTypes; //accepted media types
             AM_MEDIA_TYPE m_connectedType;
-            QString m_name;
             IMemAllocator *m_memAlloc;
         };