From 65ba2c18a9a3ec82331c0ccab47edc8e252192df Mon Sep 17 00:00:00 2001 From: Thierry Bastian Date: Mon, 10 Aug 2009 13:45:24 +0200 Subject: Fixed an assert that could happen when the mediaSource is deleted When using streaming, it could happen that the last reference to the MediaSource is in another thread. So the objects are destroyed from another thread. In which case we would delete QObject (ioDevice) in another thread. That is fixed by calling deleteLater which will ensure that they are deleted in their own thread. Note: there was a nother assert that could happen due to a race condition in the worker thread. That is also fixed with this patch. Reviewed-by: jbache --- src/3rdparty/phonon/ds9/mediaobject.cpp | 55 ++++++++++-------------------- src/3rdparty/phonon/ds9/mediaobject.h | 1 - src/3rdparty/phonon/phonon/mediasource.cpp | 8 +++-- 3 files changed, 24 insertions(+), 40 deletions(-) diff --git a/src/3rdparty/phonon/ds9/mediaobject.cpp b/src/3rdparty/phonon/ds9/mediaobject.cpp index 10782c2..b163ad4 100644 --- a/src/3rdparty/phonon/ds9/mediaobject.cpp +++ b/src/3rdparty/phonon/ds9/mediaobject.cpp @@ -57,24 +57,6 @@ namespace Phonon { } - WorkerThread::Work WorkerThread::dequeueWork() - { - QMutexLocker locker(&m_mutex); - if (m_finished) { - return Work(); - } - Work ret = m_queue.dequeue(); - - //we ensure to have the wait condition in the right state - if (m_queue.isEmpty()) { - m_waitCondition.reset(); - } else { - m_waitCondition.set(); - } - - return ret; - } - void WorkerThread::run() { while (m_finished == false) { @@ -88,11 +70,6 @@ namespace Phonon } DWORD result = ::WaitForMultipleObjects(count, handles, FALSE, INFINITE); if (result == WAIT_OBJECT_0) { - if (m_finished) { - //that's the end of the thread execution - return; - } - handleTask(); } else { //this is the event management @@ -199,22 +176,25 @@ namespace Phonon void WorkerThread::handleTask() { - const Work w = dequeueWork(); - - if (m_finished) { + QMutexLocker locker(&m_mutex); + if (m_finished || m_queue.isEmpty()) { return; } - HRESULT hr = S_OK; + const Work w = m_queue.dequeue(); - { - QMutexLocker locker(&m_mutex); - m_currentRender = w.graph; - m_currentRenderId = w.id; + //we ensure to have the wait condition in the right state + if (m_queue.isEmpty()) { + m_waitCondition.reset(); + } else { + m_waitCondition.set(); } + HRESULT hr = S_OK; + + m_currentRender = w.graph; + m_currentRenderId = w.id; if (w.task == ReplaceGraph) { - QMutexLocker locker(&m_mutex); int index = -1; for(int i = 0; i < FILTER_COUNT; ++i) { if (m_graphHandle[i].graph == w.oldGraph) { @@ -237,6 +217,9 @@ namespace Phonon m_graphHandle[index].handle = h; } } else if (w.task == Render) { + //we need to unlock here because the use might trigger a call to abort + //which uses the same mutex + locker.unlock(); if (w.filter) { //let's render pins w.graph->AddFilter(w.filter, 0); @@ -255,6 +238,7 @@ namespace Phonon if (hr != E_ABORT) { emit asyncRenderFinished(w.id, hr, w.graph); } + locker.relock(); } else if (w.task == Seek) { //that's a seekrequest ComPointer mediaSeeking(w.graph, IID_IMediaSeeking); @@ -327,11 +311,8 @@ namespace Phonon } } - { - QMutexLocker locker(&m_mutex); - m_currentRender = Graph(); - m_currentRenderId = 0; - } + m_currentRender = Graph(); + m_currentRenderId = 0; } void WorkerThread::abortCurrentRender(qint16 renderId) diff --git a/src/3rdparty/phonon/ds9/mediaobject.h b/src/3rdparty/phonon/ds9/mediaobject.h index 2c34ffc..fe52604 100644 --- a/src/3rdparty/phonon/ds9/mediaobject.h +++ b/src/3rdparty/phonon/ds9/mediaobject.h @@ -135,7 +135,6 @@ namespace Phonon }; QList decoders; //for the state change requests }; - Work dequeueWork(); void handleTask(); Graph m_currentRender; diff --git a/src/3rdparty/phonon/phonon/mediasource.cpp b/src/3rdparty/phonon/phonon/mediasource.cpp index 0a21c87..c003af9 100644 --- a/src/3rdparty/phonon/phonon/mediasource.cpp +++ b/src/3rdparty/phonon/phonon/mediasource.cpp @@ -140,8 +140,12 @@ MediaSourcePrivate::~MediaSourcePrivate() { #ifndef QT_NO_PHONON_ABSTRACTMEDIASTREAM if (autoDelete) { - delete stream; - delete ioDevice; + //here we use deleteLater because this object + //might be destroyed from another thread + if (stream) + stream->deleteLater(); + if (ioDevice) + ioDevice->deleteLater(); } #endif //QT_NO_PHONON_ABSTRACTMEDIASTREAM } -- cgit v0.12