uvw::Resource::data yields data's ownership to handlers

[stefanofiorentino]

Hi, neither bugs or issues overhere, but questions arose about user_data ownership by uvw::Resource (during a brief chat with colleagues). Is it possibile to pass data to an handler, avoiding to share ownership with it?

E.g.

TEST(Context, lifetime_for_issue_post) {
    auto context = std::make_unique<context_t>();
    auto loop = uvw::Loop::getDefault();
    {
        auto async = loop->resource<uvw::AsyncHandle>();
        async->data(std::shared_ptr<void>(context.get()));
        async->on<uvw::AsyncEvent>([](auto const &, auto &hndl) {
            hndl.close();
        });
        async->send();
        loop->run();
    }
}

Even if this code looks good, and run well in some platform, it yields an invalid free (because the delete is called two times, one from the unique and one from the user_data shared_ptr deallocator).

The following works well (as expected)

TEST(Context, lifetime_for_issue_post) {
    auto context = std::make_shared<context_t>();
    auto loop = uvw::Loop::getDefault();
    {
        auto async = loop->resource<uvw::AsyncHandle>();
        async->data(std::shared_ptr<void>(context));
        async->on<uvw::AsyncEvent>([](auto const &, auto &hndl) {
            hndl.close();
        });
        async->send();
        loop->run();
    }
}

Currently, is the unique to shared translation my only option?

[skypjack]

I'll try to figure out if there is a way to make it more user friendly. Afaik, we can just turn it into an std::unique_ptr<void, deleter> and that's all. I don't see any issue but for breaking backward compatibility. Am I wrong?

[stefanofiorentino]

I don't want to imperil lib's backwards, I'm just trying to figure out if exist a way to keep my context instance unique_ptr and avoid invalid free at the end of the program. unique_ptr::get() is not the right way to pass away for sure. I was reviewing Herb Sutter's C++11 elements: If you know another object is going to outlive you and you want to observe it, use a (non-owning) raw pointer.

class node {
 vector<unique_ptr<node>> children;
 node* parent;
public:
 :::
};

but maybe is not the solution to my "issue".

[skypjack]

if exist a way to keep my context instance unique_ptr and avoid invalid free at the end of the program

Oh, well, then you can probably use the aliasing constructor from shared pointers for that.

[stefanofiorentino]

shared_ptr aliasing constructor seems to solve the "issue" elegantly:

TEST(Context, lifetime_for_issue_post) {
    auto context = std::make_unique<context_t>();
    auto loop = uvw::Loop::getDefault();
    {
        auto async = loop->resource<uvw::AsyncHandle>();
        async->data(std::shared_ptr<void>(std::make_shared<context_t>(*context), context.get()));
        async->on<uvw::AsyncEvent>([](auto const &, auto &hndl) {
            hndl.close();
        });
        async->send();
        loop->run();
    }
}

Thanks @skypjack Maybe I'll work on adding a data() overload if needed.

[stefanofiorentino]

Even though it works, doesn't leak and doesn't double free, I'm not sure to push it to the main repo because using context.get() might mislead people to a wrongly usage of unique_ptr. Tell me what you think.

[skypjack]

I think that the aliasing constructor of a shared pointer is really an arcane feature of the standard library. It's meant to solve very particular problems. Most of the C++ devs don't even know it! I'm not sure that putting a dedicate API in the handle makes sense. One can still get around the problem but to do that he/she must know what's going on at least.

[stefanofiorentino]

Definitely agree. I'll write an helper function to do this sort of stuff. Thanks!