From cfc1f596760c1997b39bdfe8a43b4089da685988 Mon Sep 17 00:00:00 2001 From: kh Date: Mon, 20 Apr 2015 16:40:21 +0200 Subject: Fix "Too many open files" error. The current code did create all files in advance, exceeding the open file limit on big downloads. Now we create the file once we write to it. Task-number: QTIFW-662 Change-Id: I9fe019e08342cbfb14bf564ad00b045cc03b6661 Reviewed-by: Jarek Kobus --- src/libs/installer/downloadfiletask.cpp | 106 +++++++++++++++----------------- src/libs/installer/downloadfiletask_p.h | 28 ++++++--- 2 files changed, 67 insertions(+), 67 deletions(-) (limited to 'src/libs/installer') diff --git a/src/libs/installer/downloadfiletask.cpp b/src/libs/installer/downloadfiletask.cpp index d5ab2acda..f308315c9 100644 --- a/src/libs/installer/downloadfiletask.cpp +++ b/src/libs/installer/downloadfiletask.cpp @@ -1,3 +1,4 @@ + /************************************************************************** ** ** Copyright (C) 2015 The Qt Company Ltd. @@ -34,11 +35,9 @@ #include "downloadfiletask.h" #include "downloadfiletask_p.h" -#include "observer.h" #include #include -#include #include #include #include @@ -62,16 +61,10 @@ Downloader::Downloader() Downloader::~Downloader() { m_nam.disconnect(); - foreach (QNetworkReply *const reply, m_downloads.keys()) { - reply->disconnect(); - reply->abort(); - reply->deleteLater(); - } - - foreach (const Data &data, m_downloads.values()) { - data.file->close(); - delete data.file; - delete data.observer; + for (const auto &pair : m_downloads) { + pair.first->disconnect(); + pair.first->abort(); + pair.first->deleteLater(); } } @@ -119,7 +112,34 @@ void Downloader::onReadyRead() if (!reply) return; - const Data &data = m_downloads[reply]; + Data &data = *m_downloads[reply]; + if (!data.file) { + std::unique_ptr file = Q_NULLPTR; + const QString target = data.taskItem.target(); + if (target.isEmpty()) { + std::unique_ptr tmp(new QTemporaryFile); + tmp->setAutoRemove(false); + file = std::move(tmp); + } else { + std::unique_ptr tmp(new QFile(target)); + file = std::move(tmp); + } + + if (file->exists() && (!QFileInfo(file->fileName()).isFile())) { + m_futureInterface->reportException(TaskException(tr("Target file '%1' already exists " + "but is not a file.").arg(file->fileName()))); + return; + } + + if (!file->open(QIODevice::WriteOnly | QIODevice::Truncate)) { + //: %2 is a sentence describing the error + m_futureInterface->reportException(TaskException(tr("Could not open target '%1' for " + "write. Error: %2.").arg(file->fileName(), file->errorString()))); + return; + } + data.file = std::move(file); + } + if (!data.file->isOpen()) { //: %2 is a sentence describing the error. m_futureInterface->reportException( @@ -154,8 +174,8 @@ void Downloader::onReadyRead() data.observer->addCheckSumData(buffer.data(), read); int progress = m_finished * 100; - foreach (const Data &data, m_downloads.values()) - progress += data.observer->progressValue(); + for (const auto &pair : m_downloads) + progress += pair.second->observer->progressValue(); if (!reply->attribute(QNetworkRequest::RedirectionTargetAttribute).isValid()) { m_futureInterface->setProgressValueAndText(progress / m_items.count(), data.observer->progressText()); @@ -165,18 +185,16 @@ void Downloader::onReadyRead() void Downloader::onFinished(QNetworkReply *reply) { - const Data &data = m_downloads[reply]; - const QString filename = data.file->fileName(); + Data &data = *m_downloads[reply]; + const QString filename = data.file ? data.file->fileName() : QString(); if (!m_futureInterface->isCanceled()) { if (reply->attribute(QNetworkRequest::RedirectionTargetAttribute).isValid()) { const QUrl url = reply->url() .resolved(reply->attribute(QNetworkRequest::RedirectionTargetAttribute).toUrl()); const QList redirects = m_redirects.values(reply); if (!redirects.contains(url)) { - data.file->close(); - data.file->remove(); - delete data.file; - delete data.observer; + if (data.file) + data.file->remove(); FileTaskItem taskItem = data.taskItem; taskItem.insert(TaskRole::SourceFile, url.toString()); @@ -186,7 +204,7 @@ void Downloader::onFinished(QNetworkReply *reply) m_redirects.insertMulti(redirectReply, redirect); m_redirects.insertMulti(redirectReply, url); - m_downloads.remove(reply); + m_downloads.erase(reply); m_redirects.remove(reply); reply->deleteLater(); return; @@ -214,16 +232,12 @@ void Downloader::onFinished(QNetworkReply *reply) } m_futureInterface->reportResult(FileTaskResult(filename, data.observer->checkSum(), data.taskItem)); - data.file->close(); - delete data.file; - delete data.observer; - - m_downloads.remove(reply); + m_downloads.erase(reply); m_redirects.remove(reply); reply->deleteLater(); m_finished++; - if (m_downloads.isEmpty() || m_futureInterface->isCanceled()) { + if (m_downloads.empty() || m_futureInterface->isCanceled()) { m_futureInterface->reportFinished(); emit finished(); // emit finished, so the event loop can shutdown } @@ -239,7 +253,7 @@ void Downloader::onError(QNetworkReply::NetworkError error) return; // already handled by onAuthenticationRequired if (reply) { - const Data &data = m_downloads[reply]; + const Data &data = *m_downloads[reply]; //: %2 is a sentence describing the error m_futureInterface->reportException( TaskException(tr("Network error while downloading '%1': %2.").arg( @@ -266,17 +280,17 @@ void Downloader::onDownloadProgress(qint64 bytesReceived, qint64 bytesTotal) Q_UNUSED(bytesReceived) QNetworkReply *const reply = qobject_cast(sender()); if (reply) { - const Data &data = m_downloads[reply]; + const Data &data = *m_downloads[reply]; data.observer->setBytesToTransfer(bytesTotal); } } void Downloader::onAuthenticationRequired(QNetworkReply *reply, QAuthenticator *authenticator) { - if (!authenticator || !reply || !m_downloads.contains(reply)) + if (!authenticator || !reply || m_downloads.find(reply) == m_downloads.cend()) return; - FileTaskItem *item = &m_downloads[reply].taskItem; + FileTaskItem *item = &m_downloads[reply]->taskItem; const QAuthenticator auth = item->value(TaskRole::Authenticator).value(); if (auth.user().isEmpty()) { AuthenticationRequiredException e(AuthenticationRequiredException::Type::Server, @@ -327,33 +341,9 @@ QNetworkReply *Downloader::startDownload(const FileTaskItem &item) return 0; } - QScopedPointer file; - const QString target = item.target(); - if (target.isEmpty()) { - QTemporaryFile *tmp = new QTemporaryFile; - tmp->setAutoRemove(false); - file.reset(tmp); - } else { - file.reset(new QFile(target)); - } - - if (file->exists() && (!QFileInfo(file->fileName()).isFile())) { - m_futureInterface->reportException( - TaskException(tr("Target file '%1' already exists but is not a file.").arg( - file->fileName()))); - return 0; - } - - if (!file->open(QIODevice::WriteOnly | QIODevice::Truncate)) { - //: %2 is a sentence describing the error - m_futureInterface->reportException( - TaskException(tr("Could not open target '%1' for write. Error: %2.").arg( - file->fileName(), file->errorString()))); - return 0; - } - QNetworkReply *reply = m_nam.get(QNetworkRequest(source)); - m_downloads.insert(reply, Data(file.take(), new FileTaskObserver(QCryptographicHash::Sha1), item)); + std::unique_ptr data(new Data(item)); + m_downloads[reply] = std::move(data); connect(reply, SIGNAL(readyRead()), this, SLOT(onReadyRead())); connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, diff --git a/src/libs/installer/downloadfiletask_p.h b/src/libs/installer/downloadfiletask_p.h index d91d335b3..a2f2969aa 100644 --- a/src/libs/installer/downloadfiletask_p.h +++ b/src/libs/installer/downloadfiletask_p.h @@ -36,30 +36,40 @@ #define DOWNLOADFILETASK_P_H #include "downloadfiletask.h" +#include +#include #include #include #include +#include +#include + QT_BEGIN_NAMESPACE -class QFile; class QSslError; QT_END_NAMESPACE namespace QInstaller { -class FileTaskObserver; - struct Data { + Q_DISABLE_COPY(Data) + Data() - : file(0), observer(0) {} - Data(QFile *f, FileTaskObserver *o, const FileTaskItem &fti) - : file(f), observer(o), taskItem(fti) + : file(Q_NULLPTR) + , observer(Q_NULLPTR) {} - QFile *file; - FileTaskObserver *observer; + + Data(const FileTaskItem &fti) + : taskItem(fti) + , file(Q_NULLPTR) + , observer(new FileTaskObserver(QCryptographicHash::Sha1)) + {} + FileTaskItem taskItem; + std::unique_ptr file; + std::unique_ptr observer; }; class Downloader : public QObject @@ -98,8 +108,8 @@ private: int m_finished; QNetworkAccessManager m_nam; QList m_items; - QHash m_downloads; QMultiHash m_redirects; + std::unordered_map> m_downloads; }; } // namespace QInstaller -- cgit v1.2.3