From f2801d47758f108c4ee229c5ab5ee9b46eb8b652 Mon Sep 17 00:00:00 2001 From: John Preston Date: Tue, 15 Aug 2017 18:59:40 +0300 Subject: [PATCH] Fix possible crash in file download after error. Regression was introduced in 2fa2fa41c5. In file download failed handler we suggest to try to load the file once again to the same location. After some changes we started to forget filename before failed handler. That resulted in large files loading to memory instead of hard drive. Add a precondition in FileLoader to prevent such bugs in the future. --- Telegram/SourceFiles/config.h | 4 --- .../SourceFiles/media/media_clip_ffmpeg.cpp | 3 +- .../SourceFiles/media/media_clip_reader.cpp | 4 ++- .../SourceFiles/storage/file_download.cpp | 34 ++++++++++--------- Telegram/SourceFiles/storage/file_download.h | 10 ++++-- .../SourceFiles/storage/localimageloader.cpp | 11 ++++-- Telegram/SourceFiles/structs.cpp | 10 ++++-- 7 files changed, 47 insertions(+), 29 deletions(-) diff --git a/Telegram/SourceFiles/config.h b/Telegram/SourceFiles/config.h index bacf4a43a..633c4e4a6 100644 --- a/Telegram/SourceFiles/config.h +++ b/Telegram/SourceFiles/config.h @@ -93,13 +93,9 @@ enum { AudioVoiceMsgUpdateView = 100, // 100ms AudioVoiceMsgChannels = 2, // stereo AudioVoiceMsgBufferSize = 256 * 1024, // 256 Kb buffers (1.3 - 3.0 secs) - AudioVoiceMsgInMemory = 2 * 1024 * 1024, // 2 Mb audio is hold in memory and auto loaded - StickerInMemory = 2 * 1024 * 1024, // 2 Mb stickers hold in memory, auto loaded and displayed inline StickerMaxSize = 2048, // 2048x2048 is a max image size for sticker - AnimationInMemory = 10 * 1024 * 1024, // 10 Mb gif and mp4 animations held in memory while playing - MaxZoomLevel = 7, // x8 ZoomToScreenLevel = 1024, // just constant diff --git a/Telegram/SourceFiles/media/media_clip_ffmpeg.cpp b/Telegram/SourceFiles/media/media_clip_ffmpeg.cpp index 26002c3db..512564ead 100644 --- a/Telegram/SourceFiles/media/media_clip_ffmpeg.cpp +++ b/Telegram/SourceFiles/media/media_clip_ffmpeg.cpp @@ -22,6 +22,7 @@ Copyright (c) 2014-2017 John Preston, https://desktop.telegram.org #include "media/media_audio.h" #include "media/media_child_ffmpeg_loader.h" +#include "storage/file_download.h" namespace Media { namespace Clip { @@ -454,7 +455,7 @@ bool FFMpegReaderImplementation::isGifv() const { if (_hasAudioStream) { return false; } - if (dataSize() > AnimationInMemory) { + if (dataSize() > Storage::kMaxAnimationInMemory) { return false; } if (_codecContext->codec_id != AV_CODEC_ID_H264) { diff --git a/Telegram/SourceFiles/media/media_clip_reader.cpp b/Telegram/SourceFiles/media/media_clip_reader.cpp index 2c15e5e46..2f6ae3b2c 100644 --- a/Telegram/SourceFiles/media/media_clip_reader.cpp +++ b/Telegram/SourceFiles/media/media_clip_reader.cpp @@ -20,6 +20,8 @@ Copyright (c) 2014-2017 John Preston, https://desktop.telegram.org */ #include "media/media_clip_reader.h" +#include "storage/file_download.h" + extern "C" { #include #include @@ -484,7 +486,7 @@ public: } bool init() { - if (_data.isEmpty() && QFileInfo(_location->name()).size() <= AnimationInMemory) { + if (_data.isEmpty() && QFileInfo(_location->name()).size() <= Storage::kMaxAnimationInMemory) { QFile f(_location->name()); if (f.open(QIODevice::ReadOnly)) { _data = f.readAll(); diff --git a/Telegram/SourceFiles/storage/file_download.cpp b/Telegram/SourceFiles/storage/file_download.cpp index 301f42739..ac1dc1391 100644 --- a/Telegram/SourceFiles/storage/file_download.cpp +++ b/Telegram/SourceFiles/storage/file_download.cpp @@ -116,12 +116,13 @@ WebLoadMainManager *_webLoadMainManager = nullptr; FileLoader::FileLoader(const QString &toFile, int32 size, LocationType locationType, LoadToCacheSetting toCache, LoadFromCloudSetting fromCloud, bool autoLoading) : _downloader(&Auth().downloader()) , _autoLoading(autoLoading) -, _file(toFile) -, _fname(toFile) +, _filename(toFile) +, _file(_filename) , _toCache(toCache) , _fromCloud(fromCloud) , _size(size) , _locationType(locationType) { + Expects(!_filename.isEmpty() || (_size <= Storage::kMaxFileInMemory)); } QByteArray FileLoader::imageFormat(const QSize &shrinkBox) const { @@ -162,11 +163,11 @@ int32 FileLoader::fullSize() const { } bool FileLoader::setFileName(const QString &fileName) { - if (_toCache != LoadToCacheAsWell || !_fname.isEmpty()) { - return fileName.isEmpty() || (fileName == _fname); + if (_toCache != LoadToCacheAsWell || !_filename.isEmpty()) { + return fileName.isEmpty() || (fileName == _filename); } - _fname = fileName; - _file.setFileName(_fname); + _filename = fileName; + _file.setFileName(_filename); return true; } @@ -232,7 +233,7 @@ void FileLoader::localLoaded(const StorageImageSaved &result, const QByteArray & _imagePixmap = imagePixmap; } _localStatus = LocalLoaded; - if (!_fname.isEmpty() && _toCache == LoadToCacheAsWell) { + if (!_filename.isEmpty() && _toCache == LoadToCacheAsWell) { if (!_fileIsOpen) _fileIsOpen = _file.open(QIODevice::WriteOnly); if (!_fileIsOpen) { cancel(true); @@ -268,7 +269,7 @@ void FileLoader::start(bool loadFirst, bool prior) { return; } - if (!_fname.isEmpty() && _toCache == LoadToFileOnly && !_fileIsOpen) { + if (!_filename.isEmpty() && _toCache == LoadToFileOnly && !_fileIsOpen) { _fileIsOpen = _file.open(QIODevice::WriteOnly); if (!_fileIsOpen) { return cancel(true); @@ -379,8 +380,6 @@ void FileLoader::cancel(bool fail) { _file.remove(); } _data = QByteArray(); - _fname = QString(); - _file.setFileName(_fname); if (fail) { emit failed(this, started); @@ -388,6 +387,9 @@ void FileLoader::cancel(bool fail) { emit progress(this); } + _filename = QString(); + _file.setFileName(_filename); + loadNext(); } @@ -693,7 +695,7 @@ void mtpFileLoader::partLoaded(int offset, base::const_byte_span bytes) { } else { if (offset > 100 * 1024 * 1024) { // Debugging weird out of memory crashes. - auto info = QString("offset: %1, size: %2, cancelled: %3, finished: %4, filename: '%5', tocache: %6, fromcloud: %7, data: %8, fullsize: %9").arg(offset).arg(bytes.size()).arg(Logs::b(_cancelled)).arg(Logs::b(_finished)).arg(_fname).arg(int(_toCache)).arg(int(_fromCloud)).arg(_data.size()).arg(_size); + auto info = QString("offset: %1, size: %2, cancelled: %3, finished: %4, filename: '%5', tocache: %6, fromcloud: %7, data: %8, fullsize: %9").arg(offset).arg(bytes.size()).arg(Logs::b(_cancelled)).arg(Logs::b(_finished)).arg(_filename).arg(int(_toCache)).arg(int(_fromCloud)).arg(_data.size()).arg(_size); info += QString(", locationtype: %1, inqueue: %2, localstatus: %3").arg(int(_locationType)).arg(Logs::b(_inQueue)).arg(int(_localStatus)); SignalHandlers::setCrashAnnotation("DebugInfo", info); } @@ -723,7 +725,7 @@ void mtpFileLoader::partLoaded(int offset, base::const_byte_span bytes) { _lastComplete = true; } if (_sentRequests.empty() && _cdnUncheckedParts.empty() && (_lastComplete || (_size && _nextRequestOffset >= _size))) { - if (!_fname.isEmpty() && (_toCache == LoadToCacheAsWell)) { + if (!_filename.isEmpty() && (_toCache == LoadToCacheAsWell)) { if (!_fileIsOpen) _fileIsOpen = _file.open(QIODevice::WriteOnly); if (!_fileIsOpen) { return cancel(true); @@ -745,8 +747,8 @@ void mtpFileLoader::partLoaded(int offset, base::const_byte_span bytes) { Local::writeImage(storageKey(*_urlLocation), StorageImageSaved(_data)); } else if (_locationType != UnknownFileLocation) { // audio, video, document auto mkey = mediaKey(_locationType, _dcId, _id, _version); - if (!_fname.isEmpty()) { - Local::writeFileLocation(mkey, FileLocation(_fname)); + if (!_filename.isEmpty()) { + Local::writeFileLocation(mkey, FileLocation(_filename)); } if (_toCache == LoadToCacheAsWell) { if (_locationType == DocumentFileLocation) { @@ -925,7 +927,7 @@ void webFileLoader::onFinished(const QByteArray &data) { } else { _data = data; } - if (!_fname.isEmpty() && (_toCache == LoadToCacheAsWell)) { + if (!_filename.isEmpty() && (_toCache == LoadToCacheAsWell)) { if (!_fileIsOpen) _fileIsOpen = _file.open(QIODevice::WriteOnly); if (!_fileIsOpen) { return cancel(true); @@ -1126,7 +1128,7 @@ bool WebLoadManager::handleReplyResult(webFileLoaderPrivate *loader, WebReplyPro } if (result == WebReplyProcessProgress) { - if (loader->size() > AnimationInMemory) { + if (loader->size() > Storage::kMaxFileInMemory) { LOG(("API Error: too large file is loaded to cache: %1").arg(loader->size())); result = WebReplyProcessError; } diff --git a/Telegram/SourceFiles/storage/file_download.h b/Telegram/SourceFiles/storage/file_download.h index d53299312..d79dc1b30 100644 --- a/Telegram/SourceFiles/storage/file_download.h +++ b/Telegram/SourceFiles/storage/file_download.h @@ -25,6 +25,11 @@ Copyright (c) 2014-2017 John Preston, https://desktop.telegram.org namespace Storage { +constexpr auto kMaxFileInMemory = 10 * 1024 * 1024; // 10 MB max file could be hold in memory +constexpr auto kMaxVoiceInMemory = 2 * 1024 * 1024; // 2 MB audio is hold in memory and auto loaded +constexpr auto kMaxStickerInMemory = 2 * 1024 * 1024; // 2 MB stickers hold in memory, auto loaded and displayed inline +constexpr auto kMaxAnimationInMemory = kMaxFileInMemory; // 10 MB gif and mp4 animations held in memory while playing + class Downloader final { public: Downloader(); @@ -100,7 +105,7 @@ public: QByteArray imageFormat(const QSize &shrinkBox = QSize()) const; QPixmap imagePixmap(const QSize &shrinkBox = QSize()) const; QString fileName() const { - return _fname; + return _filename; } float64 currentProgress() const; virtual int32 currentOffset(bool includeSkipped = false) const = 0; @@ -165,8 +170,8 @@ protected: void loadNext(); virtual bool loadPart() = 0; + QString _filename; QFile _file; - QString _fname; bool _fileIsOpen = false; LoadToCacheSetting _toCache; @@ -328,7 +333,6 @@ class WebLoadManager : public QObject { Q_OBJECT public: - WebLoadManager(QThread *thread); #ifndef TDESKTOP_DISABLE_NETWORK_PROXY diff --git a/Telegram/SourceFiles/storage/localimageloader.cpp b/Telegram/SourceFiles/storage/localimageloader.cpp index b2d1f51ff..028c1154c 100644 --- a/Telegram/SourceFiles/storage/localimageloader.cpp +++ b/Telegram/SourceFiles/storage/localimageloader.cpp @@ -28,6 +28,7 @@ Copyright (c) 2014-2017 John Preston, https://desktop.telegram.org #include "mainwindow.h" #include "lang/lang_keys.h" #include "boxes/confirm_box.h" +#include "storage/file_download.h" namespace { @@ -512,8 +513,14 @@ void FileLoadTask::process() { } QByteArray thumbFormat = "JPG"; - int32 thumbQuality = 87; - if (!isAnimation && filemime == stickerMime && w > 0 && h > 0 && w <= StickerMaxSize && h <= StickerMaxSize && filesize < StickerInMemory) { + auto thumbQuality = 87; + if (!isAnimation + && filemime == stickerMime + && w > 0 + && h > 0 + && w <= StickerMaxSize + && h <= StickerMaxSize + && filesize < Storage::kMaxStickerInMemory) { attributes.push_back(MTP_documentAttributeSticker(MTP_flags(0), MTP_string(""), MTP_inputStickerSetEmpty(), MTPMaskCoords())); thumbFormat = "webp"; thumbname = qsl("thumb.webp"); diff --git a/Telegram/SourceFiles/structs.cpp b/Telegram/SourceFiles/structs.cpp index 78a72c4e6..cc3365231 100644 --- a/Telegram/SourceFiles/structs.cpp +++ b/Telegram/SourceFiles/structs.cpp @@ -1679,7 +1679,11 @@ void DocumentData::setattributes(const QVector &attributes } } if (type == StickerDocument) { - if (dimensions.width() <= 0 || dimensions.height() <= 0 || dimensions.width() > StickerMaxSize || dimensions.height() > StickerMaxSize || size > StickerInMemory) { + if (dimensions.width() <= 0 + || dimensions.height() <= 0 + || dimensions.width() > StickerMaxSize + || dimensions.height() > StickerMaxSize + || !saveToCache()) { type = FileDocument; _additional = nullptr; } @@ -1687,7 +1691,9 @@ void DocumentData::setattributes(const QVector &attributes } bool DocumentData::saveToCache() const { - return (type == StickerDocument) || (isAnimation() && size < AnimationInMemory) || (voice() && size < AudioVoiceMsgInMemory); + return (type == StickerDocument && size < Storage::kMaxStickerInMemory) + || (isAnimation() && size < Storage::kMaxAnimationInMemory) + || (voice() && size < Storage::kMaxVoiceInMemory); } void DocumentData::forget() {