kotatogram/kotatogram-desktop
kotatogram/kotatogram-desktop
1
0
Fork 0
Code Issues Releases 45 Packages Wiki Activity

Using plain mutex for Clip readers access serialize, not read-write.

Browse source 
This is an attempt to fix some strange crash reports in write-access
to a mutable QAtomicInt through a const_iterator in ReaderPointers.
This commit is contained in:
John Preston 2016-09-25 21:05:47 +03:00
parent 947963d5d1
commit 042c9fc23d
2 changed files with 26 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
Telegram/SourceFiles/media/media_clip_reader.cpp
View file

@ -588,15 +588,12 @@ void Manager::start(Reader *reader) {
} }
void Manager::update(Reader *reader) { void Manager::update(Reader *reader) {
QReadLocker lock(&_readerPointersMutex); QMutexLocker lock(&_readerPointersMutex);
auto i = _readerPointers.constFind(reader); auto i = _readerPointers.find(reader);
if (i == _readerPointers.cend()) { if (i == _readerPointers.cend()) {
lock.unlock(); _readerPointers.insert(reader, QAtomicInt(1));
QWriteLocker lock(&_readerPointersMutex);
_readerPointers.insert(reader, MutableAtomicInt(1));
} else { } else {
i->v.storeRelease(1); i->storeRelease(1);
} }
emit processDelayed(); emit processDelayed();
} }
@ -604,13 +601,13 @@ void Manager::update(Reader *reader) {
void Manager::stop(Reader *reader) { void Manager::stop(Reader *reader) {
if (!carries(reader)) return; if (!carries(reader)) return;
QWriteLocker lock(&_readerPointersMutex); QMutexLocker lock(&_readerPointersMutex);
_readerPointers.remove(reader); _readerPointers.remove(reader);
emit processDelayed(); emit processDelayed();
} }
bool Manager::carries(Reader *reader) const { bool Manager::carries(Reader *reader) const {
QReadLocker lock(&_readerPointersMutex); QMutexLocker lock(&_readerPointersMutex);
return _readerPointers.contains(reader); return _readerPointers.contains(reader);
} }
@ -629,19 +626,13 @@ Manager::ReaderPointers::const_iterator Manager::constUnsafeFindReaderPointer(Re
} }
bool Manager::handleProcessResult(ReaderPrivate *reader, ProcessResult result, uint64 ms) { bool Manager::handleProcessResult(ReaderPrivate *reader, ProcessResult result, uint64 ms) {
QReadLocker lock(&_readerPointersMutex); QMutexLocker lock(&_readerPointersMutex);
auto it = constUnsafeFindReaderPointer(reader); auto it = unsafeFindReaderPointer(reader);
if (result == ProcessResult::Error) { if (result == ProcessResult::Error) {
if (it != _readerPointers.cend()) { if (it != _readerPointers.cend()) {
lock.unlock(); it.key()->error();
QWriteLocker lock(&_readerPointersMutex); emit callback(it.key(), it.key()->threadIndex(), NotificationReinit);
_readerPointers.erase(it);
auto i = unsafeFindReaderPointer(reader);
if (i != _readerPointers.cend()) {
i.key()->error();
emit callback(i.key(), i.key()->threadIndex(), NotificationReinit);
_readerPointers.erase(i);
}
} }
return false; return false;
} else if (result == ProcessResult::Finished) { } 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. // 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) { if (!reader->_autoPausedGif && reader->_mode == Reader::Mode::Gif && result == ProcessResult::Repaint) {
int32 ishowing, iprevious; int32 ishowing, iprevious;
Reader::Frame *showing = it.key()->frameToShow(&ishowing), *previous = it.key()->frameToWriteNext(false, &iprevious); auto showing = it.key()->frameToShow(&ishowing), previous = it.key()->frameToWriteNext(false, &iprevious);
t_assert(previous != 0 && showing != 0 && ishowing >= 0 && iprevious >= 0); 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 > 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)) { if (reader->_frames[ishowing].when + WaitBeforeGifPause < ms || (reader->_frames[iprevious].when && previous->displayed.loadAcquire() <= 0)) {
reader->_autoPausedGif = true; reader->_autoPausedGif = true;
@ -675,7 +666,7 @@ bool Manager::handleProcessResult(ReaderPrivate *reader, ProcessResult result, u
} }
if (result == ProcessResult::Started || result == ProcessResult::CopyFrame) { if (result == ProcessResult::Started || result == ProcessResult::CopyFrame) {
t_assert(reader->_frame >= 0); t_assert(reader->_frame >= 0);
Reader::Frame *frame = it.key()->_frames + reader->_frame; auto frame = it.key()->_frames + reader->_frame;
frame->clear(); frame->clear();
frame->pix = reader->frame()->pix; frame->pix = reader->frame()->pix;
frame->original = reader->frame()->original; frame->original = reader->frame()->original;
@ -710,8 +701,8 @@ Manager::ResultHandleState Manager::handleResult(ReaderPrivate *reader, ProcessR
if (result == ProcessResult::Repaint) { if (result == ProcessResult::Repaint) {
{ {
QReadLocker lock(&_readerPointersMutex); QMutexLocker lock(&_readerPointersMutex);
ReaderPointers::const_iterator it = constUnsafeFindReaderPointer(reader); auto it = constUnsafeFindReaderPointer(reader);
if (it != _readerPointers.cend()) { if (it != _readerPointers.cend()) {
int32 index = 0; int32 index = 0;
Reader *r = it.key(); Reader *r = it.key();
@ -742,9 +733,9 @@ void Manager::process() {
bool checkAllReaders = false; bool checkAllReaders = false;
uint64 ms = getms(), minms = ms + 86400 * 1000ULL; uint64 ms = getms(), minms = ms + 86400 * 1000ULL;
{ {
QReadLocker lock(&_readerPointersMutex); QMutexLocker lock(&_readerPointersMutex);
for (auto it = _readerPointers.begin(), e = _readerPointers.end(); it != e; ++it) { 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); auto i = _readers.find(it.key()->_private);
if (i == _readers.cend()) { if (i == _readers.cend()) {
_readers.insert(it.key()->_private, 0); _readers.insert(it.key()->_private, 0);
@ -759,9 +750,9 @@ void Manager::process() {
i.key()->resumeVideo(ms); i.key()->resumeVideo(ms);
} }
} }
Reader::Frame *frame = it.key()->frameToWrite(); auto frame = it.key()->frameToWrite();
if (frame) it.key()->_private->_request = frame->request; if (frame) it.key()->_private->_request = frame->request;
it->v.storeRelease(0); it->storeRelease(0);
} }
} }
checkAllReaders = (_readers.size() > _readerPointers.size()); checkAllReaders = (_readers.size() > _readerPointers.size());
@ -787,8 +778,8 @@ void Manager::process() {
i.value() = (ms + 86400 * 1000ULL); i.value() = (ms + 86400 * 1000ULL);
} }
} else if (checkAllReaders) { } else if (checkAllReaders) {
QReadLocker lock(&_readerPointersMutex); QMutexLocker lock(&_readerPointersMutex);
ReaderPointers::const_iterator it = constUnsafeFindReaderPointer(reader); auto it = constUnsafeFindReaderPointer(reader);
if (it == _readerPointers.cend()) { if (it == _readerPointers.cend()) {
_loadLevel.fetchAndAddRelaxed(-1 * (reader->_width > 0 ? reader->_width * reader->_height : AverageGifSize)); _loadLevel.fetchAndAddRelaxed(-1 * (reader->_width > 0 ? reader->_width * reader->_height : AverageGifSize));
delete reader; delete reader;
@ -820,8 +811,8 @@ void Manager::finish() {
void Manager::clear() { void Manager::clear() {
{ {
QWriteLocker lock(&_readerPointersMutex); QMutexLocker lock(&_readerPointersMutex);
for (ReaderPointers::iterator it = _readerPointers.begin(), e = _readerPointers.end(); it != e; ++it) { for (auto it = _readerPointers.begin(), e = _readerPointers.end(); it != e; ++it) {
it.key()->_private = nullptr; it.key()->_private = nullptr;
} }
_readerPointers.clear(); _readerPointers.clear();

9
Telegram/SourceFiles/media/media_clip_reader.h
View file

@ -211,14 +211,9 @@ private:
void clear(); void clear();
QAtomicInt _loadLevel; QAtomicInt _loadLevel;
struct MutableAtomicInt { using ReaderPointers = QMap<Reader*, QAtomicInt>;
MutableAtomicInt(int value) : v(value) {
}
mutable QAtomicInt v;
};
typedef QMap<Reader*, MutableAtomicInt> ReaderPointers;
ReaderPointers _readerPointers; ReaderPointers _readerPointers;
mutable QReadWriteLock _readerPointersMutex; mutable QMutex _readerPointersMutex;
ReaderPointers::const_iterator constUnsafeFindReaderPointer(ReaderPrivate *reader) const; ReaderPointers::const_iterator constUnsafeFindReaderPointer(ReaderPrivate *reader) const;
ReaderPointers::iterator unsafeFindReaderPointer(ReaderPrivate *reader); ReaderPointers::iterator unsafeFindReaderPointer(ReaderPrivate *reader);