#9711 closed Bugs (fixed)
future continuation called twice
Reported by: | anonymous | Owned by: | viboes |
---|---|---|---|
Milestone: | Boost 1.56.0 | Component: | thread |
Version: | Boost 1.55.0 | Severity: | Problem |
Keywords: | futures | Cc: | ikispal@… |
Description
I know it is still experimental, but I found a trivial bug in future::then().
If used with the deferred launch policy the continuation is called twice:
The first time it is called in promise::set_XXX() or future::then() as it should be, but it is called again in the get() method of the future returned by the then() method.
You can reproduce the problem by running the attached code.
Attachments (1)
Change History (15)
by , 9 years ago
Attachment: | boost_future.cpp added |
---|
comment:1 by , 9 years ago
Cc: | added |
---|
comment:2 by , 9 years ago
comment:3 by , 9 years ago
What is even worse: if the future returned by then() is not void, then:
- the first call of its get() will call the continuation a second time (as described above) and will return with the result of this erroneous second invocation!
- the second call of its get() will return with an empty (default constructed?) value!
comment:4 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Thanks for catching this issue. I have not tested enough deferred futures with continuations.
comment:5 by , 9 years ago
After reading more carefully the ticket and the example code, the single call must be on the call to
futr2.get();
as the then has been called with a deferred policy, and not on the promise set_value().
comment:6 by , 9 years ago
Component: | threads → thread |
---|---|
Milestone: | To Be Determined → Boost 1.56.0 |
Just commenting the call to execute in future_deferred_continuation_shared_state should fix the issue.
virtual void launch_continuation(boost::unique_lock<boost::mutex>& ) { //execute(lk); }
follow-up: 8 comment:7 by , 9 years ago
If you want to start the continuation in the get() of futr2 that would mean that this will not start the continuation at all:
auto futr1 = promise1.get_future(); auto futr2 = futr1.then({}); promise1.set_value();
Is this intended? (Please note that the only get() call here is inside the continuation itself.) What about this:
auto futr1 = promise1.get_future(); auto futrLast = futr1.then({}).then({}).then({}); promise1.set_value();
Do I always have to keep track which future is at the end of the line and call its get()? My main use-case would be to not block at all, but this way I am forced to call get() and thus block in it.
comment:8 by , 9 years ago
Replying to anonymous:
If you want to start the continuation in the get() of futr2 that would mean that this will not start the continuation at all:
auto futr1 = promise1.get_future(); auto futr2 = futr1.then({}); promise1.set_value();
Here you create a continuation that is asynchronously called when the futr1 is ready and so it will be called independently of whether you get the value of futr2 or not. You example used deferred
auto futr2 = futr.then(boost::launch::deferred, ...);
so that the continuation should be called only when you call futr2.get().
Is this intended?
This is how I understand it. Maybe you can disagree.
(Please note that the only get() call here is inside the continuation itself.) What about this:
auto futr1 = promise1.get_future(); auto futrLast = futr1.then({}).then({}).then({}); promise1.set_value();Do I always have to keep track which future is at the end of the line and call its get()?
The last one is stored in futrLast. What i sthe problem?
My main use-case would be to not block at all, but this way I am forced to call get() and thus block in it.
Then you must use launch::async.
comment:9 by , 9 years ago
comment:10 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:11 by , 8 years ago
Commit d2a65d242751cece17e28b7cc6d7bfb395e900e8 completely disables functionality of the .then(launch::deferred, ...) continuation...
I will open a separate ticket with the test code to reproduce the issue.
comment:12 by , 8 years ago
I have added test for deferred then and every thing is OK https://github.com/boostorg/thread/commit/3e28ea806c412c697f134fdaacbccc9b64213426
comment:13 by , 8 years ago
Ok. I see now that I was using it differently and possibly wrong way. In the old implementation 'deferred' continuation was called synchronously from the thread that sets satisfies the future. And I am relying on this functionality. However deferred means that it should be called when feature value is retrieved, which now looks correct, but it is not that obvious when you start with future continuations as it is not described anywhere.
Is there any way to achieve the functionality of the sync call from the 'satisfier' side?
The second time the continuation is called an empty (invalid) future is passed to it. (I suppose the guts of the future was stolen by move() during the previous call.)