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

Ambiguity of nested hpx::future<void>'s. #2667

Closed
dmarce1 opened this issue May 31, 2017 · 8 comments · Fixed by #2673
Closed

Ambiguity of nested hpx::future<void>'s. #2667

dmarce1 opened this issue May 31, 2017 · 8 comments · Fixed by #2673

Comments

@dmarce1
Copy link
Member

dmarce1 commented May 31, 2017

When future < T > is created using a function returning a future < T > instead of a T, the outer future is not made ready until the inner future is complete.

However, when a futue< void > is created using a function returning a future< void > instead of void, the outer future is made ready as soon as the inner future is returned - which could be (and often is) before the inner future is made ready.

Example:

hpx::future<void> fut = hpx::async( []() {
     do_work();
     return hpx::async( []() {
        do_more_work();     
     };
});

Given the behavior of nested non-void futures, one would expect fut would not be made ready until after do_more_work() is executed. Apparently (from bugs I have fixed in Octo-tiger) this is not the case. Instead fut is made ready when the inner future is created and returned, which is (usually) before do_more_work() is complete.

I have fixed two bugs in Octo-tiger that were of this type, and each time the bugs were not readily reproducible and took many days and SUs to narrow down the problem and confirm I had fixed it.

Is this the intended behavior of nested void futures or is this a bug?

@hkaiser
Copy link
Member

hkaiser commented May 31, 2017

This is intended behavior. How could the outer future not become ready if the inner future is available?

@hkaiser
Copy link
Member

hkaiser commented May 31, 2017

Just to clarify: for the code snippet you showed, the outermost future should not become ready before do_more_work() finished executing, however here:

hpx::future<hpx::future<void>> fut = hpx::async( []() {
     do_work();
     return hpx::async( []() {
        do_more_work();     
     };
});

the returned outer future will become ready as soon as the inner async has returned. In other words, the automatic unwrapping of future<future<void>> to future<void> should make sure that the collapsed future becomes ready only once the original inner future has become ready.

@dmarce1
Copy link
Member Author

dmarce1 commented May 31, 2017

There must be a bug then, because I don't think that's what is happening. I will try and reproduce it with a smaller code sample.

@eschnett
Copy link
Contributor

You might be able to create a succinct test case that uses exceptions. If the outer future is ready too early, then any exceptions thrown by the inner future have no place to go any more.

@preejackie
Copy link

preejackie commented May 31, 2017 via email

@hkaiser
Copy link
Member

hkaiser commented May 31, 2017

@Praveenv98 The original code Dominic showed above exposes this behavior.

@dmarce1
Copy link
Member Author

dmarce1 commented May 31, 2017

I am having trouble reproducing this behavior with a simple code, and the Octotiger bug I thought I had fixed by explicitly getting the inner futures before return appears to be popping back up again. (Though it takes longer before it happens). At this point I'm no longer 100% convinced the HPX bug I think is there actually exists, it may still be a bug in my own code.

@hkaiser
Copy link
Member

hkaiser commented Jun 1, 2017

@dmarce1 You were right after all. The current code allows to implicitly convert a future<future<T>> (where T != void) to a future<void> which possibly leads to problems as this operation does not unwrap the nested future but just discards the result value of the outer future.

Doing the same for a future<future<void>> --> future<void> is correctly performing the required unwrap, however.

I will create a patch which inhibits the former conversion at compile time.

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.

4 participants