From ea1d6712fe11f59ee9a89cc1cfec3aaa114a5233 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Mon, 12 Apr 2010 13:10:20 -0700 Subject: [PATCH] Fix a race condition in TimedEventQueue, an event may be cancelled while we're waiting for its scheduled time to come in which case we'd be removing it from the queue twice. Change-Id: I4e42e318fd5373d1f352f54027d4bf823126266d related-to-bug: 2585276 --- media/libstagefright/TimedEventQueue.cpp | 43 ++++++++++++++++++++------ media/libstagefright/include/TimedEventQueue.h | 2 ++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/media/libstagefright/TimedEventQueue.cpp b/media/libstagefright/TimedEventQueue.cpp index e62d501a3e78..3de8c1d85971 100644 --- a/media/libstagefright/TimedEventQueue.cpp +++ b/media/libstagefright/TimedEventQueue.cpp @@ -19,6 +19,7 @@ #define __STDC_LIMIT_MACROS #include +//#define LOG_NDEBUG 0 #define LOG_TAG "TimedEventQueue" #include @@ -169,6 +170,8 @@ void TimedEventQueue::cancelEvents( mQueueHeadChangedCondition.signal(); } + LOGV("cancelling event %d", (*it).event->eventID()); + (*it).event->setEventID(0); it = mQueue.erase(it); @@ -229,14 +232,16 @@ void TimedEventQueue::threadEntry() { mQueueNotEmptyCondition.wait(mLock); } - List::iterator it; + event_id eventID = 0; for (;;) { if (mQueue.empty()) { // The only event in the queue could have been cancelled // while we were waiting for its scheduled time. break; } - it = mQueue.begin(); + + List::iterator it = mQueue.begin(); + eventID = (*it).event->eventID(); now_us = getRealTimeUs(); int64_t when_us = (*it).realtime_us; @@ -276,18 +281,38 @@ void TimedEventQueue::threadEntry() { } } - if (mQueue.empty()) { - continue; - } + // The event w/ this id may have been cancelled while we're + // waiting for its trigger-time, in that case + // removeEventFromQueue_l will return NULL. + // Otherwise, the QueueItem will be removed + // from the queue and the referenced event returned. + event = removeEventFromQueue_l(eventID); + } + + if (event != NULL) { + // Fire event with the lock NOT held. + event->fire(this, now_us); + } + } +} - event = (*it).event; +sp TimedEventQueue::removeEventFromQueue_l( + event_id id) { + for (List::iterator it = mQueue.begin(); + it != mQueue.end(); ++it) { + if ((*it).event->eventID() == id) { + sp event = (*it).event; event->setEventID(0); + mQueue.erase(it); - } - // Fire event with the lock NOT held. - event->fire(this, now_us); + return event; + } } + + LOGW("Event %d was not found in the queue, already cancelled?", id); + + return NULL; } } // namespace android diff --git a/media/libstagefright/include/TimedEventQueue.h b/media/libstagefright/include/TimedEventQueue.h index 21eade370815..11f844c6d3f2 100644 --- a/media/libstagefright/include/TimedEventQueue.h +++ b/media/libstagefright/include/TimedEventQueue.h @@ -121,6 +121,8 @@ private: static void *ThreadWrapper(void *me); void threadEntry(); + sp removeEventFromQueue_l(event_id id); + TimedEventQueue(const TimedEventQueue &); TimedEventQueue &operator=(const TimedEventQueue &); }; -- 2.11.0