continuable icon indicating copy to clipboard operation
continuable copied to clipboard

How do you run final cleanup with a chain?

Open geiseri opened this issue 3 years ago • 8 comments

@Naios

I am currently using https://github.com/xhawk18/promise-cpp and would like to move to continuable since I am using native asio. One thing that is holding me back is the Promise::finally(FUNC_ON_FINALLY on_finally) method. I use this because I need to have a few cleanup operations that can only run after the entire chain has been resolved. I was having issues finding a way to make this with next(...) without duplicating code in both functions. Am I missing something?

geiseri avatar Nov 09 '22 14:11 geiseri

@geiseri Could you maybe provide a code example what you have tried so far?

Naios avatar Nov 09 '22 22:11 Naios

This is the original code: My issue is that this chain resolves at a future time and I need inst to live on.

promise::Promise  getResponse(Type symbolType, std::string symbol) {
  auto inst = registry.get(symbolType); // This is a shared ptr 
  return inst->resolveScore(symbol).finally([inst ]() {(void)inst;});
}

I got this far, but it crashes with inst going away somewhere along the way.

cti::continuable<StrategyResponse> getResponse(Type symbolType, std::string symbol) {
  auto inst = registry.get(symbolType); // This is a shared ptr 
  return inst->resolveScore(symbol)
      .then([inst](StrategyResponse res) {
          (void)inst;
          return cti::make_ready_continuable(res);
      });
}

If I pull the inst out of the function and pass it in, it works just fine, but the goal of the method in the first place was to hide the registry.get(...) part from the public API. I think the issue might be that the finally runs after the whole promise chain is resolved, where then will release it right away.

Does that make more sense?

geiseri avatar Nov 12 '22 02:11 geiseri

could it be that you're allowing the cti::continuable returned from getResponse to be destroyed prematurely? how are you calling getResponse ?

Spongman avatar Nov 15 '22 18:11 Spongman

It is when I am still in a .then(...) call. Is that a different instance of the continuable? Or is that different? I was under the impression that inst would stay in scope until the resolution of the final handler in the chain. That being said, it could be that there is another issue going on that was hidden by the promise::Promise implementation.

geiseri avatar Nov 15 '22 21:11 geiseri

It is when I am still in a .then(...) call. Is that a different instance of the continuable?

Not quite sure what you mean by this. It might be helpful to include more context to how you’re calling this function, and how you’re handling the lifetime of the continuable that it returns. There’s nothing wrong with the code you’ve shown here, so the error must be elsewhere.

Maybe put a breakpoint in your ‘inst’ object’s destructor and look at the stack trace.

A couple of other things:

  • if your resolveScore function throws, then you'll need to handle the exceptions in a .fail clause (in addition to the .then clause)
  • the return cti::make_ready_continuable(res); is unnecessary, you can just return res; from a .then callback.
  • you may be able to avoid doing this entirely if you just make your inst object derive from enable_shared_from_this, and make sure that whatever inner async operations are performed within resolveScore hold onto a shared_ptr reference to this.

Spongman avatar Nov 16 '22 00:11 Spongman

I second what @Spongman says. Tracing the destructor of your shared_ptr object through a debugger is probably your best option.

Additionally I have to add that the callable that you pass to then, fail, or next stays valid until it can be potentially called, but not post that. Callables are destructed in generally immediatly when they are no longer needed for the chain.

So if you require your handle/shared_ptr to stay alive afterwards you need to reference it in the handler you are using the handle in.

Btw. you can also use next as an "catch all" handler which could be used to implement something like finally.

Naios avatar Nov 16 '22 07:11 Naios

Okay, I think I found the issue. Originally the code used .finally inside of the method that returned the continuation. It looks like all .finally handlers were run after the entire chain was resolved. It seems like I need to move that shared pointer creation into the caller, and pass it into the method. I was wondering though. I am using asio there is an ability to add values that are passed to the completion handler via the executor. I did not see an obvious method to convert back and forth from a continuable back to the underlying asio executor. I assume that is not easy since a continuable may or may not have initially been created from there. I can always return a tuple with both data values though.

geiseri avatar Dec 05 '22 19:12 geiseri

There is no mechanism to allow a true finally handler that runs when the entire chain has ended because in the process of building the chain you might always add further continuations.

The executor is getting passed down into the continuation and therefore is not accessible anymore from outside.

Naios avatar Dec 05 '22 19:12 Naios