From 042c9fc23dca4ddc04cce41769b2ec6a799d87f4 Mon Sep 17 00:00:00 2001 From: John Preston Date: Sun, 25 Sep 2016 21:05:47 +0300 Subject: [PATCH] Using plain mutex for Clip readers access serialize, not read-write. This is an attempt to fix some strange crash reports in write-access to a mutable QAtomicInt through a const_iterator in ReaderPointers. --- .../SourceFiles/media/media_clip_reader.cpp | 57 ++++++++----------- .../SourceFiles/media/media_clip_reader.h | 9 +-- 2 files changed, 26 insertions(+), 40 deletions(-) diff --git a/Telegram/SourceFiles/media/media_clip_reader.cpp b/Telegram/SourceFiles/media/media_clip_reader.cpp index 49ddc2f06e..2ed2aa0e03 100644 --- a/Telegram/SourceFiles/media/media_clip_reader.cpp +++ b/Telegram/SourceFiles/media/media_clip_reader.cpp @@ -588,15 +588,12 @@ void Manager::start(Reader *reader) { } void Manager::update(Reader *reader) { - QReadLocker lock(&_readerPointersMutex); - auto i = _readerPointers.constFind(reader); + QMutexLocker lock(&_readerPointersMutex); + auto i = _readerPointers.find(reader); if (i == _readerPointers.cend()) { - lock.unlock(); - - QWriteLocker lock(&_readerPointersMutex); - _readerPointers.insert(reader, MutableAtomicInt(1)); + _readerPointers.insert(reader, QAtomicInt(1)); } else { - i->v.storeRelease(1); + i->storeRelease(1); } emit processDelayed(); } @@ -604,13 +601,13 @@ void Manager::update(Reader *reader) { void Manager::stop(Reader *reader) { if (!carries(reader)) return; - QWriteLocker lock(&_readerPointersMutex); + QMutexLocker lock(&_readerPointersMutex); _readerPointers.remove(reader); emit processDelayed(); } bool Manager::carries(Reader *reader) const { - QReadLocker lock(&_readerPointersMutex); + QMutexLocker lock(&_readerPointersMutex); return _readerPointers.contains(reader); } @@ -629,19 +626,13 @@ Manager::ReaderPointers::const_iterator Manager::constUnsafeFindReaderPointer(Re } bool Manager::handleProcessResult(ReaderPrivate *reader, ProcessResult result, uint64 ms) { - QReadLocker lock(&_readerPointersMutex); - auto it = constUnsafeFindReaderPointer(reader); + QMutexLocker lock(&_readerPointersMutex); + auto it = unsafeFindReaderPointer(reader); if (result == ProcessResult::Error) { if (it != _readerPointers.cend()) { - lock.unlock(); - QWriteLocker lock(&_readerPointersMutex); - - auto i = unsafeFindReaderPointer(reader); - if (i != _readerPointers.cend()) { - i.key()->error(); - emit callback(i.key(), i.key()->threadIndex(), NotificationReinit); - _readerPointers.erase(i); - } + it.key()->error(); + emit callback(it.key(), it.key()->threadIndex(), NotificationReinit); + _readerPointers.erase(it); } return false; } else if (result == ProcessResult::Finished) { @@ -663,8 +654,8 @@ bool Manager::handleProcessResult(ReaderPrivate *reader, ProcessResult result, u // See if we need to pause GIF because it is not displayed right now. if (!reader->_autoPausedGif && reader->_mode == Reader::Mode::Gif && result == ProcessResult::Repaint) { int32 ishowing, iprevious; - Reader::Frame *showing = it.key()->frameToShow(&ishowing), *previous = it.key()->frameToWriteNext(false, &iprevious); - t_assert(previous != 0 && showing != 0 && ishowing >= 0 && iprevious >= 0); + auto showing = it.key()->frameToShow(&ishowing), previous = it.key()->frameToWriteNext(false, &iprevious); + t_assert(previous != nullptr && showing != nullptr && ishowing >= 0 && iprevious >= 0); if (reader->_frames[ishowing].when > 0 && showing->displayed.loadAcquire() <= 0) { // current frame was not shown if (reader->_frames[ishowing].when + WaitBeforeGifPause < ms || (reader->_frames[iprevious].when && previous->displayed.loadAcquire() <= 0)) { reader->_autoPausedGif = true; @@ -675,7 +666,7 @@ bool Manager::handleProcessResult(ReaderPrivate *reader, ProcessResult result, u } if (result == ProcessResult::Started || result == ProcessResult::CopyFrame) { t_assert(reader->_frame >= 0); - Reader::Frame *frame = it.key()->_frames + reader->_frame; + auto frame = it.key()->_frames + reader->_frame; frame->clear(); frame->pix = reader->frame()->pix; frame->original = reader->frame()->original; @@ -710,8 +701,8 @@ Manager::ResultHandleState Manager::handleResult(ReaderPrivate *reader, ProcessR if (result == ProcessResult::Repaint) { { - QReadLocker lock(&_readerPointersMutex); - ReaderPointers::const_iterator it = constUnsafeFindReaderPointer(reader); + QMutexLocker lock(&_readerPointersMutex); + auto it = constUnsafeFindReaderPointer(reader); if (it != _readerPointers.cend()) { int32 index = 0; Reader *r = it.key(); @@ -742,9 +733,9 @@ void Manager::process() { bool checkAllReaders = false; uint64 ms = getms(), minms = ms + 86400 * 1000ULL; { - QReadLocker lock(&_readerPointersMutex); + QMutexLocker lock(&_readerPointersMutex); for (auto it = _readerPointers.begin(), e = _readerPointers.end(); it != e; ++it) { - if (it->v.loadAcquire() && it.key()->_private != nullptr) { + if (it->loadAcquire() && it.key()->_private != nullptr) { auto i = _readers.find(it.key()->_private); if (i == _readers.cend()) { _readers.insert(it.key()->_private, 0); @@ -759,9 +750,9 @@ void Manager::process() { i.key()->resumeVideo(ms); } } - Reader::Frame *frame = it.key()->frameToWrite(); + auto frame = it.key()->frameToWrite(); if (frame) it.key()->_private->_request = frame->request; - it->v.storeRelease(0); + it->storeRelease(0); } } checkAllReaders = (_readers.size() > _readerPointers.size()); @@ -787,8 +778,8 @@ void Manager::process() { i.value() = (ms + 86400 * 1000ULL); } } else if (checkAllReaders) { - QReadLocker lock(&_readerPointersMutex); - ReaderPointers::const_iterator it = constUnsafeFindReaderPointer(reader); + QMutexLocker lock(&_readerPointersMutex); + auto it = constUnsafeFindReaderPointer(reader); if (it == _readerPointers.cend()) { _loadLevel.fetchAndAddRelaxed(-1 * (reader->_width > 0 ? reader->_width * reader->_height : AverageGifSize)); delete reader; @@ -820,8 +811,8 @@ void Manager::finish() { void Manager::clear() { { - QWriteLocker lock(&_readerPointersMutex); - for (ReaderPointers::iterator it = _readerPointers.begin(), e = _readerPointers.end(); it != e; ++it) { + QMutexLocker lock(&_readerPointersMutex); + for (auto it = _readerPointers.begin(), e = _readerPointers.end(); it != e; ++it) { it.key()->_private = nullptr; } _readerPointers.clear(); diff --git a/Telegram/SourceFiles/media/media_clip_reader.h b/Telegram/SourceFiles/media/media_clip_reader.h index d2a3f7be60..90dd5e3dd3 100644 --- a/Telegram/SourceFiles/media/media_clip_reader.h +++ b/Telegram/SourceFiles/media/media_clip_reader.h @@ -211,14 +211,9 @@ private: void clear(); QAtomicInt _loadLevel; - struct MutableAtomicInt { - MutableAtomicInt(int value) : v(value) { - } - mutable QAtomicInt v; - }; - typedef QMap ReaderPointers; + using ReaderPointers = QMap; ReaderPointers _readerPointers; - mutable QReadWriteLock _readerPointersMutex; + mutable QMutex _readerPointersMutex; ReaderPointers::const_iterator constUnsafeFindReaderPointer(ReaderPrivate *reader) const; ReaderPointers::iterator unsafeFindReaderPointer(ReaderPrivate *reader);