Skip to content

Commit a63ac43

Browse files
authored
bug fix: avoid close uv handle opened by hiredis. (#648)
1 parent ffba366 commit a63ac43

File tree

2 files changed

+32
-12
lines changed

2 files changed

+32
-12
lines changed

src/sw/redis++/event_loop.cpp

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
#include "sw/redis++/event_loop.h"
1818
#include <cassert>
19+
#include <chrono>
20+
#include <thread>
1921
#include <hiredis/adapters/libuv.h>
2022
#include "sw/redis++/async_connection.h"
2123

@@ -208,26 +210,42 @@ void EventLoop::LoopDeleter::operator()(uv_loop_t *loop) const {
208210
// How to correctly close an event loop:
209211
// https://stackoverflow.com/questions/25615340/closing-libuv-handles-correctly
210212
// TODO: do we need to call this? Since we always has 2 async_t handles.
211-
if (uv_loop_close(loop) == 0) {
212-
delete loop;
213+
//if (uv_loop_close(loop) == 0) {
214+
// delete loop;
215+
//
216+
// return;
217+
//}
213218

214-
return;
215-
}
219+
assert(loop->data != nullptr);
216220

217221
uv_walk(loop,
218-
[](uv_handle_t *handle, void *) {
222+
[](uv_handle_t *handle, void *arg) {
219223
if (handle != nullptr) {
220-
// We don't need to release handle's memory in close callback,
221-
// since we'll release the memory in EventLoop's destructor.
222-
uv_close(handle, nullptr);
224+
auto *event_loop = static_cast<EventLoop *>(arg);
225+
226+
assert(event_loop != nullptr);
227+
228+
if (handle == reinterpret_cast<uv_handle_t *>(event_loop->_event_async.get()) ||
229+
handle == reinterpret_cast<uv_handle_t *>(event_loop->_stop_async.get())) {
230+
// We don't need to release handle's memory in close callback,
231+
// since we'll release the memory in EventLoop's destructor.
232+
uv_close(handle, nullptr);
233+
}
223234
}
224235
},
225-
nullptr);
236+
loop->data);
226237

227238
// Ensure uv_walk's callback to be called.
228239
uv_run(loop, UV_RUN_DEFAULT);
229240

230-
uv_loop_close(loop);
241+
for (auto idx = 0; idx < 10; ++idx) {
242+
if (uv_loop_close(loop) == 0) {
243+
break;
244+
}
245+
246+
// maybe hiredis does not close the handle yet? wait a while.
247+
std::this_thread::sleep_for(std::chrono::milliseconds(100));
248+
}
231249

232250
delete loop;
233251
}
@@ -271,14 +289,16 @@ EventLoop::UvAsyncUPtr EventLoop::_create_uv_async(AsyncCallback callback) {
271289
return uv_async;
272290
}
273291

274-
EventLoop::LoopUPtr EventLoop::_create_event_loop() const {
292+
EventLoop::LoopUPtr EventLoop::_create_event_loop() {
275293
auto *loop = new uv_loop_t;
276294
auto err = uv_loop_init(loop);
277295
if (err != 0) {
278296
delete loop;
279297
throw Error("failed to initialize event loop: " + _err_msg(err));
280298
}
281299

300+
loop->data = this;
301+
282302
return LoopUPtr(loop);
283303
}
284304

src/sw/redis++/event_loop.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class EventLoop {
7373
return uv_strerror(err);
7474
}
7575

76-
LoopUPtr _create_event_loop() const;
76+
LoopUPtr _create_event_loop();
7777

7878
using UvAsyncUPtr = std::unique_ptr<uv_async_t>;
7979

0 commit comments

Comments
 (0)