commit ed78f00484345565912f242222d10b0511bd891c
parent 4950bb6b83fc18ebb4eb882244988c3de643ca29
Author: cfillion <cfillion@users.noreply.github.com>
Date: Sun, 4 Jun 2017 00:06:19 -0400
download: fix a possible crash when a finished download is cancelled
Such a crash would occur when closing the Import dialog just before the
finish notification got received in the main thread.
Diffstat:
6 files changed, 40 insertions(+), 48 deletions(-)
diff --git a/src/archive.cpp b/src/archive.cpp
@@ -220,19 +220,14 @@ FileExtractor::FileExtractor(const Path &target, const ArchiveReaderPtr &reader)
setSummary("Extracting %s: " + target.join());
}
-void FileExtractor::run()
+bool FileExtractor::run()
{
- if(aborted()) {
- finish(Aborted, {"cancelled", m_path.target().join()});
- return;
- }
-
ThreadNotifier::get()->notify({this, Running});
ofstream stream;
if(!FS::open(stream, m_path.temp())) {
- finish(Failure, {FS::lastError(), m_path.temp().join()});
- return;
+ setError({FS::lastError(), m_path.temp().join()});
+ return false;
}
const int error = m_reader->extractFile(m_path.target(), stream);
@@ -240,10 +235,11 @@ void FileExtractor::run()
if(error) {
const format &msg = format("Failed to extract file (%d)") % error;
- finish(Failure, {msg.str(), m_path.target().join()});
+ setError({msg.str(), m_path.target().join()});
+ return false;
}
- else
- finish(Success);
+
+ return true;
}
size_t Archive::create(const auto_string &path, vector<string> *errors,
@@ -362,19 +358,14 @@ FileCompressor::FileCompressor(const Path &target, const ArchiveWriterPtr &write
setSummary("Compressing %s: " + target.join());
}
-void FileCompressor::run()
+bool FileCompressor::run()
{
- if(aborted()) {
- finish(Aborted, {"cancelled", m_path.join()});
- return;
- }
-
ThreadNotifier::get()->notify({this, Running});
ifstream stream;
if(!FS::open(stream, m_path)) {
- finish(Failure, {FS::lastError(), m_path.join()});
- return;
+ setError({FS::lastError(), m_path.join()});
+ return false;
}
const int error = m_writer->addFile(m_path, stream);
@@ -382,8 +373,9 @@ void FileCompressor::run()
if(error) {
const format &msg = format("Failed to compress file (%d)") % error;
- finish(Failure, {msg.str(), m_path.join()});
+ setError({msg.str(), m_path.join()});
+ return false;
}
- else
- finish(Success);
+
+ return true;
}
diff --git a/src/archive.hpp b/src/archive.hpp
@@ -65,7 +65,7 @@ public:
const TempPath &path() const { return m_path; }
bool concurrent() const override { return false; }
- void run() override;
+ bool run() override;
private:
TempPath m_path;
@@ -77,7 +77,7 @@ public:
FileCompressor(const Path &target, const ArchiveWriterPtr &);
bool concurrent() const override { return false; }
- void run() override;
+ bool run() override;
private:
Path m_path;
diff --git a/src/download.cpp b/src/download.cpp
@@ -120,18 +120,13 @@ void Download::start()
onFinish([thread] { delete thread; });
}
-void Download::run()
+bool Download::run()
{
- if(aborted()) {
- finish(Aborted, {"cancelled", m_url});
- return;
- }
-
ThreadNotifier::get()->notify({this, Running});
ostream *stream = openStream();
if(!stream)
- return;
+ return false;
curl_easy_setopt(m_ctx->m_curl, CURLOPT_URL, m_url.c_str());
curl_easy_setopt(m_ctx->m_curl, CURLOPT_PROXY, m_opts.proxy.c_str());
@@ -152,18 +147,16 @@ void Download::run()
curl_easy_setopt(m_ctx->m_curl, CURLOPT_ERRORBUFFER, errbuf);
const CURLcode res = curl_easy_perform(m_ctx->m_curl);
+ curl_slist_free_all(headers);
closeStream();
- if(aborted())
- finish(Aborted, {"aborted", m_url});
- else if(res != CURLE_OK) {
+ if(res != CURLE_OK) {
const auto err = format("%s (%d): %s") % curl_easy_strerror(res) % res % errbuf;
- finish(Failure, {err.str(), m_url});
+ setError({err.str(), m_url});
+ return false;
}
- else
- finish(Success);
- curl_slist_free_all(headers);
+ return true;
}
MemoryDownload::MemoryDownload(const string &url, const NetworkOpts &opts, int flags)
@@ -191,9 +184,10 @@ ostream *FileDownload::openStream()
{
if(FS::open(m_stream, m_path.temp()))
return &m_stream;
-
- finish(Failure, {FS::lastError(), m_path.temp().join()});
- return nullptr;
+ else {
+ setError({FS::lastError(), m_path.temp().join()});
+ return nullptr;
+ }
}
void FileDownload::closeStream()
diff --git a/src/download.hpp b/src/download.hpp
@@ -51,7 +51,7 @@ public:
void start();
bool concurrent() const override { return true; }
- void run() override;
+ bool run() override;
protected:
virtual std::ostream *openStream() = 0;
diff --git a/src/thread.cpp b/src/thread.cpp
@@ -38,7 +38,9 @@ ThreadTask::~ThreadTask()
void ThreadTask::setState(const State state)
{
- m_state = state;
+ // The task may have been aborted while the task was running or just before
+ // the finish notification got received in the main thread.
+ m_state = aborted() ? Aborted : state;
switch(state) {
case Idle:
@@ -55,9 +57,12 @@ void ThreadTask::setState(const State state)
}
}
-void ThreadTask::finish(const State state, const ErrorInfo &error)
+void ThreadTask::exec()
{
- m_error = error;
+ State state = Aborted;
+
+ if(!aborted())
+ state = run() ? Success : Failure;
ThreadNotifier::get()->notify({this, state});
};
@@ -90,7 +95,7 @@ DWORD WINAPI WorkerThread::run(void *ptr)
if(auto dl = dynamic_cast<Download *>(task))
dl->setContext(&context);
- task->run();
+ task->exec();
}
ResetEvent(thread->m_wake);
diff --git a/src/thread.hpp b/src/thread.hpp
@@ -52,11 +52,13 @@ public:
virtual ~ThreadTask();
virtual bool concurrent() const = 0;
- virtual void run() = 0;
+ virtual bool run() = 0;
+ void exec();
const std::string &summary() const { return m_summary; }
void setState(State);
State state() const { return m_state; }
+ void setError(const ErrorInfo &err) { m_error = err; }
const ErrorInfo &error() { return m_error; }
void onStart(const VoidSignal::slot_type &slot) { m_onStart.connect(slot); }
@@ -68,7 +70,6 @@ public:
protected:
void setSummary(const std::string &s) { m_summary = s; }
- void finish(State, const ErrorInfo & = {});
private:
std::string m_summary;
