Naios / continuable

C++14 asynchronous allocation aware futures (supporting then, exception handling, coroutines and connections)
https://naios.github.io/continuable/
MIT License
815 stars 44 forks source link

How do you run final cleanup with a chain? #58

Open geiseri opened 1 year ago

geiseri commented 1 year ago

@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?

Naios commented 1 year ago

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

geiseri commented 1 year ago

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?

Spongman commented 1 year ago

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

geiseri commented 1 year ago

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.

Spongman commented 1 year ago

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:

Naios commented 1 year ago

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.

geiseri commented 1 year ago

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.

Naios commented 1 year ago

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.