Skip to content

Commit 91ba631

Browse files
TrifleMichaelTrifleMichael
authored
CCDBDownloader: More error checks (#11489)
* CCDBDownloader: Libuv error checks * Formatting correction * Added check for premature uvloop closure * Added curl checks and socket open/close checks * After merge fix * Removing redundant download
1 parent 366e6bb commit 91ba631

2 files changed

Lines changed: 83 additions & 45 deletions

File tree

‎CCDB/include/CCDB/CCDBDownloader.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,11 @@ class CCDBDownloader
187187
*/
188188
bool mIsExternalLoop;
189189

190+
/**
191+
* Used in debug to detect whether uv loop closed prematurely.
192+
*/
193+
bool mIsClosing = false;
194+
190195
/**
191196
* Current amount of handles which are performed on.
192197
*/

‎CCDB/src/CCDBDownloader.cxx‎

Lines changed: 78 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
#include <CCDB/CCDBDownloader.h>
1313

1414
#include <curl/curl.h>
15-
#include <uv.h>
16-
1715
#include <unordered_map>
1816
#include <cstdio>
1917
#include <cstdlib>
@@ -26,10 +24,34 @@
2624
#include <unistd.h>
2725
#include <sys/types.h>
2826
#include <sys/socket.h>
27+
#include <fairlogger/Logger.h>
2928

3029
namespace o2::ccdb
3130
{
3231

32+
void uvErrorCheck(int code)
33+
{
34+
if (code != 0) {
35+
char buf[1000];
36+
uv_strerror_r(code, buf, 1000);
37+
LOG(error) << "CCDBDownloader: UV error - " << buf;
38+
}
39+
}
40+
41+
void curlEasyErrorCheck(CURLcode code)
42+
{
43+
if (code != CURLE_OK) {
44+
LOG(error) << "CCDBDownloader: CURL error - " << curl_easy_strerror(code);
45+
}
46+
}
47+
48+
void curlMultiErrorCheck(CURLMcode code)
49+
{
50+
if (code != CURLM_OK) {
51+
LOG(error) << "CCDBDownloader: CURL error - " << curl_multi_strerror(code);
52+
}
53+
}
54+
3355
CCDBDownloader::CCDBDownloader(uv_loop_t* uv_loop)
3456
{
3557
if (uv_loop) {
@@ -43,8 +65,8 @@ CCDBDownloader::CCDBDownloader(uv_loop_t* uv_loop)
4365
// Preparing timer to be used by curl
4466
mTimeoutTimer = new uv_timer_t();
4567
mTimeoutTimer->data = this;
46-
uv_loop_init(mUVLoop);
47-
uv_timer_init(mUVLoop, mTimeoutTimer);
68+
uvErrorCheck(uv_loop_init(mUVLoop));
69+
uvErrorCheck(uv_timer_init(mUVLoop, mTimeoutTimer));
4870
mHandleMap[(uv_handle_t*)mTimeoutTimer] = true;
4971

5072
// Preparing curl handle
@@ -54,35 +76,36 @@ CCDBDownloader::CCDBDownloader(uv_loop_t* uv_loop)
5476
// uv_loop runs only when there are active handles, this handle guarantees the loop won't close immedietly after starting
5577
auto timerCheckQueueHandle = new uv_timer_t();
5678
timerCheckQueueHandle->data = this;
57-
uv_timer_init(mUVLoop, timerCheckQueueHandle);
79+
uvErrorCheck(uv_timer_init(mUVLoop, timerCheckQueueHandle));
5880
mHandleMap[(uv_handle_t*)timerCheckQueueHandle] = true;
59-
uv_timer_start(timerCheckQueueHandle, checkStopSignal, 100, 100);
81+
uvErrorCheck(uv_timer_start(timerCheckQueueHandle, checkStopSignal, 100, 100));
6082

6183
mLoopThread = new std::thread(&CCDBDownloader::runLoop, this);
6284
}
6385

6486
void CCDBDownloader::initializeMultiHandle()
6587
{
6688
mCurlMultiHandle = curl_multi_init();
67-
curl_multi_setopt(mCurlMultiHandle, CURLMOPT_SOCKETFUNCTION, handleSocket);
89+
curlMultiErrorCheck(curl_multi_setopt(mCurlMultiHandle, CURLMOPT_SOCKETFUNCTION, handleSocket));
6890
auto socketData = &mSocketData;
6991
socketData->curlm = mCurlMultiHandle;
7092
socketData->CD = this;
71-
curl_multi_setopt(mCurlMultiHandle, CURLMOPT_SOCKETDATA, socketData);
72-
curl_multi_setopt(mCurlMultiHandle, CURLMOPT_TIMERFUNCTION, startTimeout);
73-
curl_multi_setopt(mCurlMultiHandle, CURLMOPT_TIMERDATA, mTimeoutTimer);
74-
curl_multi_setopt(mCurlMultiHandle, CURLMOPT_MAX_TOTAL_CONNECTIONS, mMaxHandlesInUse);
93+
curlMultiErrorCheck(curl_multi_setopt(mCurlMultiHandle, CURLMOPT_SOCKETDATA, socketData));
94+
curlMultiErrorCheck(curl_multi_setopt(mCurlMultiHandle, CURLMOPT_TIMERFUNCTION, startTimeout));
95+
curlMultiErrorCheck(curl_multi_setopt(mCurlMultiHandle, CURLMOPT_TIMERDATA, mTimeoutTimer));
96+
curlMultiErrorCheck(curl_multi_setopt(mCurlMultiHandle, CURLMOPT_MAX_TOTAL_CONNECTIONS, mMaxHandlesInUse));
7597
}
7698

7799
CCDBDownloader::~CCDBDownloader()
78100
{
101+
mIsClosing = true;
79102
// Cleanup and close all socket timers (curl_multi_cleanup will take care of the sockets)
80103
for (auto socketTimerPair : mSocketTimerMap) {
81104
auto timer = socketTimerPair.second;
82105
if (timer->data) {
83106
delete (DataForClosingSocket*)timer->data;
84107
}
85-
uv_timer_stop(socketTimerPair.second);
108+
uvErrorCheck(uv_timer_stop(socketTimerPair.second));
86109
uv_close((uv_handle_t*)socketTimerPair.second, onUVClose);
87110
}
88111
// all timers have been closed --> so clear this map (otherwise it may get accessed in different callbacks again
@@ -102,15 +125,15 @@ CCDBDownloader::~CCDBDownloader()
102125
while (UV_EBUSY == uv_loop_close(mUVLoop)) {
103126
mCloseLoop = false;
104127
uv_walk(mUVLoop, closeHandles, this);
105-
uv_run(mUVLoop, UV_RUN_ONCE);
128+
uvErrorCheck(uv_run(mUVLoop, UV_RUN_ONCE));
106129
}
107130
delete mUVLoop;
108131
}
109132

110133
// delete timer
111134
// delete mTimeoutTimer; ---> not necessay (done elsewhere??)
112135

113-
curl_multi_cleanup(mCurlMultiHandle);
136+
curlMultiErrorCheck(curl_multi_cleanup(mCurlMultiHandle));
114137
}
115138

116139
void closeHandles(uv_handle_t* handle, void* arg)
@@ -134,7 +157,7 @@ void CCDBDownloader::checkStopSignal(uv_timer_t* handle)
134157
// Check for closing signal
135158
auto CD = (CCDBDownloader*)handle->data;
136159
if (CD->mCloseLoop) {
137-
uv_timer_stop(handle);
160+
uvErrorCheck(uv_timer_stop(handle));
138161
uv_stop(CD->mUVLoop);
139162
}
140163
CD->checkForThreadsToJoin();
@@ -145,24 +168,29 @@ void CCDBDownloader::closesocketCallback(void* clientp, curl_socket_t item)
145168
auto CD = (CCDBDownloader*)clientp;
146169
if (CD->mSocketTimerMap.find(item) != CD->mSocketTimerMap.end()) {
147170
auto timer = CD->mSocketTimerMap[item];
148-
uv_timer_stop(timer);
171+
uvErrorCheck(uv_timer_stop(timer));
149172
// we are getting rid of the uv_timer_t pointer ... so we need
150173
// to free possibly attached user data pointers as well. Counteracts action of opensocketCallback
151174
if (timer->data) {
152175
delete (DataForClosingSocket*)timer->data;
153176
}
154177
CD->mSocketTimerMap.erase(item);
155-
close(item);
178+
if (close(item) == -1) {
179+
LOG(error) << "CCDBDownloader: Socket failed to close";
180+
}
156181
}
157182
}
158183

159184
curl_socket_t opensocketCallback(void* clientp, curlsocktype purpose, struct curl_sockaddr* address)
160185
{
161186
auto CD = (CCDBDownloader*)clientp;
162187
auto sock = socket(address->family, address->socktype, address->protocol);
188+
if (sock == -1) {
189+
LOG(error) << "CCDBDownloader: Socket failed to open";
190+
}
163191

164192
CD->mSocketTimerMap[sock] = new uv_timer_t();
165-
uv_timer_init(CD->mUVLoop, CD->mSocketTimerMap[sock]);
193+
uvErrorCheck(uv_timer_init(CD->mUVLoop, CD->mSocketTimerMap[sock]));
166194
CD->mHandleMap[(uv_handle_t*)CD->mSocketTimerMap[sock]] = true;
167195

168196
auto data = new DataForClosingSocket();
@@ -187,9 +215,11 @@ void CCDBDownloader::closeSocketByTimer(uv_timer_t* handle)
187215
auto sock = data->socket;
188216

189217
if (CD->mSocketTimerMap.find(sock) != CD->mSocketTimerMap.end()) {
190-
uv_timer_stop(CD->mSocketTimerMap[sock]);
218+
uvErrorCheck(uv_timer_stop(CD->mSocketTimerMap[sock]));
191219
CD->mSocketTimerMap.erase(sock);
192-
close(sock);
220+
if (close(sock) == -1) {
221+
LOG(error) << "CCDBDownloader: Socket failed to close";
222+
}
193223

194224
delete data;
195225
}
@@ -216,7 +246,7 @@ void CCDBDownloader::curlPerform(uv_poll_t* handle, int status, int events)
216246

217247
auto context = (CCDBDownloader::curl_context_t*)handle->data;
218248

219-
curl_multi_socket_action(context->CD->mCurlMultiHandle, context->sockfd, flags, &running_handles);
249+
curlMultiErrorCheck(curl_multi_socket_action(context->CD->mCurlMultiHandle, context->sockfd, flags, &running_handles));
220250
context->CD->checkMultiInfo();
221251
}
222252

@@ -233,7 +263,7 @@ int CCDBDownloader::handleSocket(CURL* easy, curl_socket_t s, int action, void*
233263
case CURL_POLL_INOUT:
234264

235265
curl_context = socketp ? (CCDBDownloader::curl_context_t*)socketp : CD->createCurlContext(s);
236-
curl_multi_assign(socketData->curlm, s, (void*)curl_context);
266+
curlMultiErrorCheck(curl_multi_assign(socketData->curlm, s, (void*)curl_context));
237267

238268
if (action != CURL_POLL_IN) {
239269
events |= UV_WRITABLE;
@@ -243,19 +273,19 @@ int CCDBDownloader::handleSocket(CURL* easy, curl_socket_t s, int action, void*
243273
}
244274

245275
if (CD->mSocketTimerMap.find(s) != CD->mSocketTimerMap.end()) {
246-
uv_timer_stop(CD->mSocketTimerMap[s]);
276+
uvErrorCheck(uv_timer_stop(CD->mSocketTimerMap[s]));
247277
}
248278

249-
uv_poll_start(curl_context->poll_handle, events, curlPerform);
279+
uvErrorCheck(uv_poll_start(curl_context->poll_handle, events, curlPerform));
250280
break;
251281
case CURL_POLL_REMOVE:
252282
if (socketp) {
253283
if (CD->mSocketTimerMap.find(s) != CD->mSocketTimerMap.end()) {
254-
uv_timer_start(CD->mSocketTimerMap[s], closeSocketByTimer, CD->mKeepaliveTimeoutMS, 0);
284+
uvErrorCheck(uv_timer_start(CD->mSocketTimerMap[s], closeSocketByTimer, CD->mKeepaliveTimeoutMS, 0));
255285
}
256-
uv_poll_stop(((CCDBDownloader::curl_context_t*)socketp)->poll_handle);
286+
uvErrorCheck(uv_poll_stop(((CCDBDownloader::curl_context_t*)socketp)->poll_handle));
257287
CD->destroyCurlContext((CCDBDownloader::curl_context_t*)socketp);
258-
curl_multi_assign(socketData->curlm, s, nullptr);
288+
curlMultiErrorCheck(curl_multi_assign(socketData->curlm, s, nullptr));
259289
}
260290
break;
261291
default:
@@ -325,7 +355,7 @@ CCDBDownloader::curl_context_t* CCDBDownloader::createCurlContext(curl_socket_t
325355
context->sockfd = sockfd;
326356
context->poll_handle = new uv_poll_t();
327357

328-
uv_poll_init_socket(mUVLoop, context->poll_handle, sockfd);
358+
uvErrorCheck(uv_poll_init_socket(mUVLoop, context->poll_handle, sockfd));
329359
mHandleMap[(uv_handle_t*)(context->poll_handle)] = true;
330360
context->poll_handle->data = context;
331361

@@ -354,9 +384,9 @@ void CCDBDownloader::transferFinished(CURL* easy_handle, CURLcode curlCode)
354384
{
355385
mHandlesInUse--;
356386
PerformData* data;
357-
curl_easy_getinfo(easy_handle, CURLINFO_PRIVATE, &data);
387+
curlEasyErrorCheck(curl_easy_getinfo(easy_handle, CURLINFO_PRIVATE, &data));
358388

359-
curl_multi_remove_handle(mCurlMultiHandle, easy_handle);
389+
curlMultiErrorCheck(curl_multi_remove_handle(mCurlMultiHandle, easy_handle));
360390
*data->codeDestination = curlCode;
361391

362392
// If no requests left then signal finished based on type of operation
@@ -383,7 +413,7 @@ void CCDBDownloader::transferFinished(CURL* easy_handle, CURLcode curlCode)
383413

384414
// Calling timeout starts a new download if a new easy_handle was added.
385415
int running_handles;
386-
curl_multi_socket_action(mCurlMultiHandle, CURL_SOCKET_TIMEOUT, 0, &running_handles);
416+
curlMultiErrorCheck(curl_multi_socket_action(mCurlMultiHandle, CURL_SOCKET_TIMEOUT, 0, &running_handles));
387417
checkMultiInfo();
388418
}
389419

@@ -411,27 +441,27 @@ int CCDBDownloader::startTimeout(CURLM* multi, long timeout_ms, void* userp)
411441
auto timeout = (uv_timer_t*)userp;
412442

413443
if (timeout_ms < 0) {
414-
uv_timer_stop(timeout);
444+
uvErrorCheck(uv_timer_stop(timeout));
415445
} else {
416446
if (timeout_ms == 0) {
417447
timeout_ms = 1; // Calling curlTimeout when timeout = 0 could create an infinite loop
418448
}
419-
uv_timer_start(timeout, curlTimeout, timeout_ms, 0);
449+
uvErrorCheck(uv_timer_start(timeout, curlTimeout, timeout_ms, 0));
420450
}
421451
return 0;
422452
}
423453

424454
void CCDBDownloader::setHandleOptions(CURL* handle, PerformData* data)
425455
{
426-
curl_easy_setopt(handle, CURLOPT_PRIVATE, data);
427-
curl_easy_setopt(handle, CURLOPT_CLOSESOCKETFUNCTION, closesocketCallback);
428-
curl_easy_setopt(handle, CURLOPT_CLOSESOCKETDATA, this);
429-
curl_easy_setopt(handle, CURLOPT_OPENSOCKETFUNCTION, opensocketCallback);
430-
curl_easy_setopt(handle, CURLOPT_OPENSOCKETDATA, this);
456+
curlEasyErrorCheck(curl_easy_setopt(handle, CURLOPT_PRIVATE, data));
457+
curlEasyErrorCheck(curl_easy_setopt(handle, CURLOPT_CLOSESOCKETFUNCTION, closesocketCallback));
458+
curlEasyErrorCheck(curl_easy_setopt(handle, CURLOPT_CLOSESOCKETDATA, this));
459+
curlEasyErrorCheck(curl_easy_setopt(handle, CURLOPT_OPENSOCKETFUNCTION, opensocketCallback));
460+
curlEasyErrorCheck(curl_easy_setopt(handle, CURLOPT_OPENSOCKETDATA, this));
431461

432-
curl_easy_setopt(handle, CURLOPT_TIMEOUT_MS, mRequestTimeoutMS);
433-
curl_easy_setopt(handle, CURLOPT_CONNECTTIMEOUT_MS, mConnectionTimeoutMS);
434-
curl_easy_setopt(handle, CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS, mHappyEyeballsHeadstartMS);
462+
curlEasyErrorCheck(curl_easy_setopt(handle, CURLOPT_TIMEOUT_MS, mRequestTimeoutMS));
463+
curlEasyErrorCheck(curl_easy_setopt(handle, CURLOPT_CONNECTTIMEOUT_MS, mConnectionTimeoutMS));
464+
curlEasyErrorCheck(curl_easy_setopt(handle, CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS, mHappyEyeballsHeadstartMS));
435465
}
436466

437467
void CCDBDownloader::checkHandleQueue()
@@ -441,7 +471,7 @@ void CCDBDownloader::checkHandleQueue()
441471
if (mHandlesToBeAdded.size() > 0) {
442472
// Add handles without going over the limit
443473
while (mHandlesToBeAdded.size() > 0 && mHandlesInUse < mMaxHandlesInUse) {
444-
curl_multi_add_handle(mCurlMultiHandle, mHandlesToBeAdded.front());
474+
curlMultiErrorCheck(curl_multi_add_handle(mCurlMultiHandle, mHandlesToBeAdded.front()));
445475
mHandlesInUse++;
446476
mHandlesToBeAdded.erase(mHandlesToBeAdded.begin());
447477
}
@@ -451,7 +481,10 @@ void CCDBDownloader::checkHandleQueue()
451481

452482
void CCDBDownloader::runLoop()
453483
{
454-
uv_run(mUVLoop, UV_RUN_DEFAULT);
484+
uvErrorCheck(uv_run(mUVLoop, UV_RUN_DEFAULT));
485+
if (!mIsClosing) {
486+
LOG(error) << "CCDBDownloader: uvloop closed prematurely";
487+
}
455488
}
456489

457490
CURLcode CCDBDownloader::perform(CURL* handle)
@@ -545,8 +578,8 @@ void CCDBDownloader::makeLoopCheckQueueAsync()
545578
{
546579
auto asyncHandle = new uv_async_t();
547580
asyncHandle->data = this;
548-
uv_async_init(mUVLoop, asyncHandle, asyncUVHandleCheckQueue);
549-
uv_async_send(asyncHandle);
581+
uvErrorCheck(uv_async_init(mUVLoop, asyncHandle, asyncUVHandleCheckQueue));
582+
uvErrorCheck(uv_async_send(asyncHandle));
550583
}
551584

552585
} // namespace o2

0 commit comments

Comments
 (0)