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

out of order destruction #61

Open
Spongman opened this issue Dec 20, 2022 · 3 comments
Open

out of order destruction #61

Spongman opened this issue Dec 20, 2022 · 3 comments
Labels

Comments

@Spongman
Copy link

@Naios

I'm not sure if this is an issue with the continuable lib, or if I'm doing something wrong, or if I just funamentally don't understand how coroutines are supposed to work...

The issue is that the destructor for a local object in a coroutine is getting called after the co_return has 'returned' control to the caller:

https://gcc.godbolt.org/z/cT7rv93hP

#include <string>
#include <iostream>

#include <coroutine>
#include <https://gist.githubusercontent.com/Spongman/e775a7cd44246f861c146e307fea2761/raw/77b84c9f1cbb33abaecc6b43f2067bd537f092d1/continuable-63e3ed4edccfc3bcc3c8ceb12b857432eb4ee9a3-clang-fix.hpp>

struct RAII {
	std::string name_;
	RAII(std::string name) : name_{name} { std::cerr << name_ << "\n"; }
	~RAII() { std::cerr << "~" << name_ << "\n"; }
};

cti::continuable<> outer()
{
	RAII _{__FUNCTION__};
	std::cerr << __FUNCTION__ << " co_return\n";
	co_return;
}

int main() {

    outer()
        .then([]() {
            std::cerr << "then\n";
        })
        .fail([](std::exception_ptr p) {
            std::cerr << "fail\n";
        })
        .apply(cti::transforms::wait());

    return 0;
}

this outputs:

outer
outer co_return
then
~outer

i would expect that last two lines to be swapped, as it would be if this were a normal synchronous function.

maybe I'm managing the lifetime of the future incorrectly? or that just the way it's supposed to work with coroutines?


Commit Hash

master (plus my clang fix)

  • OS: linux (various)
  • Compiler and version: clang/gcc (various)
@Naios
Copy link
Owner

Naios commented Dec 21, 2022

@Spongman I'm quite certain that this behaviour is as expected with the following reasoning:

  1. The return value of outer() is part of a larger expression and therefore kept alive to the end of the entire expression.
  2. co_return performs a "final suspend" on the coroutine, therefore the coroutine is put to sleep until the coroutine_handle is destroyed. Therefore the stack inside the coroutine is kept alive.
  3. Because the coroutine_handle is destroyed when destructing the return type of outer() and not when reaching co_return, the message is printed at the end of the expression.
  4. As far as I remind, destroying the coroutine_handle from outside is neccessary to prevent a dangling handle.

@Spongman
Copy link
Author

Spongman commented Dec 21, 2022

Yeah, i figured it had something to do with the lifetime of the coroutine. i'm not familiar with the internal details, but it does seem to me like the frame should be deleted before the callback is invoked.

Here's another example that illustrates this independent of the way it's invoked:

cti::coroutine<> inner()     |  void inner()
{                            |  {
  Foo local;                 |    Foo local;
  co_return;                 |    return;
}                            |  }
                             | 
cti::coroutine<> outer()     |  void outer()
{                            |  {
  co_await inner();          |    inner();
  cerr << "inner done\n";    |    cerr << "inner done\n";
  co_return;                 |    return;
}                            |  }

In the synchronous version on the right, obviously local's destructor is called before "inner done" is printed.

However, in the async version on the left, local's destructor isn't called until after "inner done" is printed.

Regardless of the internal implementation, that just doesn't seem right to me: it means the scope rules of RAII are essentially broken in coroutines.

@Naios Naios added the bug label Dec 22, 2022
@Naios
Copy link
Owner

Naios commented Dec 22, 2022

Ok, I totally agree with you now.

I'm not sure if this is easy to fix, since in general it is not explicitly specified by the library design when the destruction of the outer continuation occurs after it was used.
Therefore, the behavior you describe may also vary depending on whether you resolve the continuable/coroutine asynchronously or synchronously.
For callable objects this behavior has never been an issue before, but for coroutine handles we should not keep the scope of the coroutine alive because this can lead to very unexpected results as you have said.

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

No branches or pull requests

2 participants