Skip to content

Commit 171d75f

Browse files
halx99halx99
authored
Fix #2548 Downloader-curl may crash or hang in download thread (#2549)
1 parent 9fb49e9 commit 171d75f

File tree

11 files changed

+214
-179
lines changed

11 files changed

+214
-179
lines changed

‎core/network/ConcurrentRefCountedBase.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#pragma once
2+
3+
#include <atomic>
4+
5+
namespace ax
6+
{
7+
class ConcurrentRefCountedBase
8+
{
9+
public:
10+
ConcurrentRefCountedBase() : _refCount(1) {}
11+
virtual ~ConcurrentRefCountedBase() {}
12+
13+
void retain() { ++_refCount; }
14+
15+
void release()
16+
{
17+
if (--_refCount == 0)
18+
delete this;
19+
}
20+
21+
int getReferenceCount() const { return _refCount; }
22+
23+
protected:
24+
std::atomic_int _refCount;
25+
};
26+
} // namespace ax

‎core/network/Downloader-curl.cpp

Lines changed: 141 additions & 117 deletions
Large diffs are not rendered by default.

‎core/network/Downloader-curl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class Scheduler;
3636

3737
namespace network
3838
{
39-
class DownloadTaskCURL;
39+
class DownloadContextCURL;
4040
class DownloaderHints;
4141
class DownloaderCURL;
4242

‎core/network/Downloader-wasm.cpp

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,18 @@ namespace ax { namespace network {
3333

3434
static int sDownloaderCounter;
3535

36-
struct DownloadTaskEmscripten : public IDownloadTask
36+
struct DownloadContextEmscripten : public IDownloadContext
3737
{
38-
explicit DownloadTaskEmscripten(unsigned int id_)
38+
explicit DownloadContextEmscripten(unsigned int id_)
3939
:id(id_)
4040
,bytesReceived(0)
4141
,fetch(NULL)
4242
{
43-
AXLOGD("Construct DownloadTaskEmscripten: {}", fmt::ptr(this));
43+
AXLOGD("Construct DownloadContextEmscripten: {}", fmt::ptr(this));
4444
}
45-
virtual ~DownloadTaskEmscripten()
45+
virtual ~DownloadContextEmscripten()
4646
{
47-
AXLOGD("Destruct DownloadTaskEmscripten: {}", fmt::ptr(this));
47+
AXLOGD("Destruct DownloadContextEmscripten: {}", fmt::ptr(this));
4848
}
4949

5050
int bytesReceived;
@@ -88,11 +88,11 @@ namespace ax { namespace network {
8888
emscripten_fetch_t *fetch = emscripten_fetch(&attr, task->requestURL.c_str());
8989
fetch->userData = this;
9090

91-
DownloadTaskEmscripten *coTask = new DownloadTaskEmscripten(fetch->id);
92-
coTask->task = task;
91+
auto context = new DownloadContextEmscripten(fetch->id);
92+
context->task = task;
9393

94-
AXLOGD("DownloaderEmscripten::createCoTask id: {}", coTask->id);
95-
_taskMap.insert(make_pair(coTask->id, coTask));
94+
AXLOGD("DownloaderEmscripten::startTask context-id: {}", context->id);
95+
_taskMap.emplace(context->id, context);
9696
}
9797

9898
void DownloaderEmscripten::onDataLoad(emscripten_fetch_t *fetch)
@@ -107,20 +107,20 @@ namespace ax { namespace network {
107107
AXLOGD("DownloaderEmscripten::onDataLoad can't find task with id: {}, size: {}", taskId, size);
108108
return;
109109
}
110-
DownloadTaskEmscripten *coTask = iter->second;
110+
auto context = iter->second;
111111
std::vector<unsigned char> buf(reinterpret_cast<const uint8_t*>(fetch->data), reinterpret_cast<const uint8_t*>(fetch->data) + size);
112112
emscripten_fetch_close(fetch);
113-
coTask->fetch = fetch = NULL;
113+
context->fetch = fetch = NULL;
114114

115115
downloader->_taskMap.erase(iter);
116-
downloader->onTaskFinish(*coTask->task,
116+
downloader->onTaskFinish(*context->task,
117117
DownloadTask::ERROR_NO_ERROR,
118118
0,
119119
"",
120120
buf
121121
);
122122

123-
coTask->task.reset();
123+
context->task.reset();
124124
}
125125

126126
void DownloaderEmscripten::onLoad(emscripten_fetch_t *fetch)
@@ -135,11 +135,11 @@ namespace ax { namespace network {
135135
AXLOGD("DownloaderEmscripten::onLoad can't find task with id: {}, size: {}", taskId, size);
136136
return;
137137
}
138-
DownloadTaskEmscripten *coTask = iter->second;
138+
auto context = iter->second;
139139
vector<unsigned char> buf;
140140
downloader->_taskMap.erase(iter);
141141

142-
string storagePath = coTask->task->storagePath;
142+
string storagePath = context->task->storagePath;
143143
int errCode = DownloadTask::ERROR_NO_ERROR;
144144
int errCodeInternal = 0;
145145
string errDescription;
@@ -197,15 +197,15 @@ namespace ax { namespace network {
197197

198198
} while (0);
199199
emscripten_fetch_close(fetch);
200-
coTask->fetch = fetch = NULL;
200+
context->fetch = fetch = NULL;
201201

202-
downloader->onTaskFinish(*coTask->task,
202+
downloader->onTaskFinish(*context->task,
203203
errCode,
204204
errCodeInternal,
205205
errDescription,
206206
buf
207207
);
208-
coTask->task.reset();
208+
context->task.reset();
209209
}
210210

211211
void DownloaderEmscripten::onProgress(emscripten_fetch_t *fetch)
@@ -227,9 +227,9 @@ namespace ax { namespace network {
227227
return;
228228
}
229229

230-
DownloadTaskEmscripten *coTask = iter->second;
231-
coTask->bytesReceived = dlNow;
232-
downloader->onTaskProgress(*coTask->task);
230+
auto context = iter->second;
231+
context->bytesReceived = dlNow;
232+
downloader->onTaskProgress(*context->task);
233233
}
234234

235235
void DownloaderEmscripten::onError(emscripten_fetch_t *fetch)
@@ -244,19 +244,19 @@ namespace ax { namespace network {
244244
AXLOGD("DownloaderEmscripten::onLoad can't find task with id: {}", taskId);
245245
return;
246246
}
247-
DownloadTaskEmscripten *coTask = iter->second;
247+
auto context = iter->second;
248248
vector<unsigned char> buf;
249249
downloader->_taskMap.erase(iter);
250-
downloader->onTaskFinish(*coTask->task,
250+
downloader->onTaskFinish(*context->task,
251251
DownloadTask::ERROR_IMPL_INTERNAL,
252252
fetch->status,
253253
fetch->statusText,
254254
buf
255255
);
256256

257257
emscripten_fetch_close(fetch);
258-
coTask->fetch = fetch = NULL;
259-
coTask->task.reset();
258+
context->fetch = fetch = NULL;
259+
context->task.reset();
260260
}
261261
}
262262
} // namespace ax::network

‎core/network/Downloader-wasm.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,23 +35,22 @@ namespace ax {
3535

3636
namespace ax { namespace network
3737
{
38-
class DownloadTaskEmscripten;
38+
class DownloadContextEmscripten;
3939

4040
class DownloaderEmscripten : public IDownloaderImpl
4141
{
4242
public:
4343
DownloaderEmscripten(const DownloaderHints& hints);
4444
virtual ~DownloaderEmscripten();
4545

46-
// virtual IDownloadTask *createCoTask(std::shared_ptr<const DownloadTask>& task) override;
4746
virtual void startTask(std::shared_ptr<DownloadTask>& task) override;
4847

4948
protected:
5049
int _id;
5150

5251
DownloaderHints hints;
5352

54-
std::unordered_map<unsigned int, DownloadTaskEmscripten*> _taskMap;
53+
std::unordered_map<unsigned int, DownloadContextEmscripten*> _taskMap;
5554

5655
static void onError(emscripten_fetch_t *fetch);
5756

‎core/network/Downloader.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ DownloadTask::~DownloadTask()
7979

8080
void DownloadTask::cancel()
8181
{
82-
if (_coTask)
83-
_coTask->cancel();
82+
if (_context)
83+
_context->cancel();
8484
}
8585

8686
////////////////////////////////////////////////////////////////////////////////

‎core/network/Downloader.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ namespace ax
3939
namespace network
4040
{
4141

42-
class IDownloadTask;
42+
class IDownloadContext;
4343
class IDownloaderImpl;
4444
class Downloader;
4545
class DownloaderCURL;
@@ -94,7 +94,7 @@ class AX_DLL DownloadTask final
9494
private:
9595
friend class Downloader;
9696
friend class DownloaderCURL;
97-
std::unique_ptr<IDownloadTask> _coTask;
97+
std::shared_ptr<IDownloadContext> _context{nullptr};
9898
};
9999

100100
class AX_DLL DownloaderHints

‎core/network/HttpRequest.h

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838

3939
#include "yasio/byte_buffer.hpp"
4040

41+
#include "ConcurrentRefCountedBase.h"
42+
4143
/**
4244
* @addtogroup network
4345
* @{
@@ -54,26 +56,6 @@ class HttpResponse;
5456

5557
typedef std::function<void(HttpClient* client, HttpResponse* response)> ccHttpRequestCallback;
5658

57-
class TSFRefCountedBase
58-
{
59-
public:
60-
TSFRefCountedBase() : _refCount(1) {}
61-
virtual ~TSFRefCountedBase() {}
62-
63-
void retain() { ++_refCount; }
64-
65-
void release()
66-
{
67-
if (--_refCount == 0)
68-
delete this;
69-
}
70-
71-
int getReferenceCount() const { return _refCount; }
72-
73-
protected:
74-
std::atomic_int _refCount;
75-
};
76-
7759
/**
7860
* Defines the object which users must packed for HttpClient::send(HttpRequest*) method.
7961
* Please refer to tests/test-cpp/Classes/ExtensionTest/NetworkTest/HttpClientTest.cpp as a sample
@@ -82,7 +64,7 @@ class TSFRefCountedBase
8264
* @lua NA
8365
*/
8466

85-
class AX_DLL HttpRequest : public TSFRefCountedBase
67+
class AX_DLL HttpRequest : public ConcurrentRefCountedBase
8668
{
8769
friend class HttpClient;
8870

‎core/network/HttpResponse.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ class HttpClient;
5252
* @since v2.0.2.
5353
* @lua NA
5454
*/
55-
class AX_DLL HttpResponse : public TSFRefCountedBase
55+
class AX_DLL HttpResponse : public ConcurrentRefCountedBase
5656
{
5757
friend class HttpClient;
5858

‎core/network/IDownloaderImpl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ namespace network
3939
{
4040
class DownloadTask;
4141

42-
class AX_DLL IDownloadTask
42+
class AX_DLL IDownloadContext
4343
{
4444
public:
45-
virtual ~IDownloadTask() {}
45+
virtual ~IDownloadContext(){}
4646
virtual void cancel() {}
4747
};
4848

0 commit comments

Comments
 (0)