jbaldwin / liblifthttp

Safe and easy to use C++17 HTTP client library.
Other
61 stars 16 forks source link

lift::client destructor infinite loop #149

Closed Radresingen closed 4 months ago

Radresingen commented 5 months ago
while (uv_loop_alive(&m_uv_loop))
{
    std::this_thread::sleep_for(1ms);
    uv_async_send(&m_uv_async); // fake a request to make sure the loop wakes up
}

this loop doesnt end. I couldnt address the problem maybe Libuv versiyon i am using.

jbaldwin commented 5 months ago

Hi @Radresingen what version of libuv are you using?

Radresingen commented 5 months ago

I have tried different versions i observed the same behaviour. Right now i am using version 1.48.0

Radresingen commented 5 months ago

Hi @jbaldwin is there any idea or plan to fix this.

jbaldwin commented 5 months ago

I probably will have some time this weekend to look at it more in depth, but do you have a test case that would reproduce this?

This code most likely would trigger if there is something still alive in the uv loop that hasn't closed, the loop above on line 138 should in theory guarantee that the loop is empty, but something is still lingering. Given a test case that you have triggering this would make it a lot easier to step through and see whats going on.

Its been a while since I've worked on this and was pretty stable the last time I really used it -- so its also possible libuv has had some changes that need to be accounted for.

Radresingen commented 5 months ago

First i just checked the code base and how to use it after that i ran the examples you provided. I havent checked that the tests are stopping and execution ends. Under the examples directory with async_simple.cpp source i can reproduce the problem. When desctructor called it's never ending.

jbaldwin commented 5 months ago

Gotcha, most likely a libuv change that is causing it, but that narrows down where to start, thanks.

jbaldwin commented 5 months ago

I'm not able to reproduce on the following configuration so I'm going to try building a custom libuv 1.48 and see if it reproduces.

ubuntu
Release:    22.04

Package: libuv1-dev
Version: 1.43.0-1ubuntu0.1
jbaldwin commented 5 months ago

I've replicated with libuv 1.48.

jbaldwin commented 5 months ago

Ok I've determined the libuv change, they no longer seem to allow you to close uv_handle_t objects off the loop thread, if you do the loop threads internal counters still say they are active so it never quits.

I introduced a new async channel to send a shutdown signal to the loop thread and now it will properly close all its uv_handle_t objects so it knows it can properly shutdown.

I've attached a PR if you'd like to take a look @Radresingen

jbaldwin commented 5 months ago

Sadly this isn't backwards compatible and breaks it on older versions of libuv :(

jbaldwin commented 5 months ago

I've managed to half port it backwards... it is working on ubuntu but not fedora :thinking:

edit: ./gif its working??

jbaldwin commented 5 months ago

@Radresingen can you checkout the branch and verify it is working for you please?

Radresingen commented 5 months ago

@jbaldwin i will try it and get back to you asap thanks.

Radresingen commented 4 months ago

@jbaldwin sorry for being late. This fixes the issue thanks for your support.

jbaldwin commented 4 months ago

Awesome, glad it's working. I'll get it merged soon and cut a new release!