Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to use component clients as action return types #2150

Closed
chinz07 opened this issue May 10, 2016 · 6 comments · Fixed by #2184
Closed

Unable to use component clients as action return types #2150

chinz07 opened this issue May 10, 2016 · 6 comments · Fixed by #2184

Comments

@chinz07
Copy link
Contributor

chinz07 commented May 10, 2016

If one is using an arbitrary component client type as the return type of an action, HPX (commit 6f3e89a) segfaults while constructing the result future. I could trace back the root of the segfault to an invalid cast in continuation.hpp via UBSan. Unfortunately, I wasn't able to pinpoint the exact location yet.

My best guess would be that one of the future specializations is called spuriously since hpx::traits::is_future is derived from boost::mpl::true_ for all component client types. This also causes a compiler error if the action is invoked directly. In this case, HPX will try to return the result as a future while the actual return type is the client type itself (basic_action.hpp, line 305).

Specializing is_future explicitly and deriving it from boost::mpl::false_ fixes both issues but I am not sure if this doesn't break other code.

The behaviour should be reproducible using the following code:

#include <hpx/hpx_init.hpp>
#include <hpx/include/actions.hpp>
#include <hpx/include/components.hpp>

#include <utility>

class foo_server : public hpx::components::component_base<foo_server>
{
};

using server_type = hpx::components::component<foo_server>;
HPX_REGISTER_COMPONENT(server_type, foo_server);

class foo : public hpx::components::client_base<foo, foo_server>
{
public:
    using base_type = hpx::components::client_base<foo, foo_server>;

    foo() = default;

    foo(hpx::future<hpx::id_type>&& id) : base_type(std::move(id))
    {
    }
};

foo get_foo()
{
    return hpx::new_<foo_server>(hpx::find_here());
}

HPX_PLAIN_ACTION(get_foo, get_foo_action);

int hpx_main(int, char*[])
{
    //Uncommenting the following line will lead to a compiler error.
    //auto result = get_foo_action()(hpx::find_here());

    auto result2 = hpx::async<get_foo_action>(hpx::find_here()).get();

    return hpx::finalize();
}

int main(int argc, char* argv[])
{
    return hpx::init(argc, argv);
}
@sithhell
Copy link
Member

sithhell commented May 10, 2016

I was fixing a lot of those recently, I wonder why this wasn't caught in
our unit tests, thanks for the report, I will investigate further!

The idea is indeed that clients should behave, semantically, like a future.
That's why the trait you mentioned returns true.

@chinz07
Copy link
Contributor Author

chinz07 commented May 13, 2016

I think that I have found the cause of the segfault. The invalid cast is on line 931 in continuation.hpp (commit ad84acb). Printing the dynamic type of the continuation yields hpx::actions::typed_continuation<foo, foo> while the code tries to cast it to hpx::actions::typed_continuation<hpx::naming::id_type const&, hpx::naming::id_type const&>.

If I am not mistaken, there is no relation between these two types which would explain the error. Since component client are just wrappers around id_type (Please correct me if I am wrong.) my guess would be that HPX unwraps the ID to transfers it and then forgets to rewrap it on the call site.

I will continue my investigation and try to find why this is not happening.

@chinz07
Copy link
Contributor Author

chinz07 commented May 14, 2016

@sithhell I have found another suspicious cast on line 336 in continuation.hpp. The dynamic type of the continuation is hpx::actions::typed_continuation<foo, foo> as expected but the code tries to cast it to hpx::actions::typed_continuation<foo, hpx::naming::id_type>.

@sithhell
Copy link
Member

sithhell commented May 14, 2016

Thanks! I fixed a lot of those recently, it's a little complicated... I
won't have time to look into that issue more closely right now, sorry.
The workaround is to return future<id_type> from the action, that worked
for me.

@hkaiser
Copy link
Member

hkaiser commented May 26, 2016

@chinz07 Please try #2184, it should fix your problem

@chinz07
Copy link
Contributor Author

chinz07 commented May 26, 2016

@hkaiser This fixes the issue for me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants