-
Notifications
You must be signed in to change notification settings - Fork 44
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
critical issue with nested continuables #65
Comments
Sorry for my late response. I have seen this issue as you created it. Thank you for your report. |
Were you able to reproduce and/or fix this? This issue is causing a bunch of use-after-free issues and segfaults for me, and it's becoming increasingly difficult to work around. |
I have taken a look at your example and I had to fix a few issues with it: #include <iostream>
#include <string>
#include <thread>
#include <list>
#include <continuable/continuable.hpp>
#include <asio/defer.hpp>
#include <asio/io_context.hpp>
#include <asio/steady_timer.hpp>
#include <coroutine>
#include <continuable/continuable.hpp>
#include <continuable/external/asio.hpp>
using namespace std;
using namespace chrono;
using namespace asio;
struct Foo {
inline static int i__ = 0;
volatile int i = i__++;
Foo() {
cout << i << " Foo()\n";
}
Foo(const Foo&) {
cout << i << " Foo(const Foo&)\n";
}
Foo(Foo&&) noexcept {
cout << i << " Foo(Foo&&)\n";
}
Foo& operator=(const Foo&) {
cout << i << " operator=(const Foo&)\n";
return *this;
}
Foo& operator=(Foo&&) noexcept {
cout << i << " operator=(Foo&&)\n";
return *this;
}
~Foo() {
cout << i << " ~Foo()\n";
i = -i;
}
};
io_context ctx;
std::list<asio::steady_timer> timers;
int main() {
auto workGuard = make_work_guard(ctx);
thread thd{[&]() {
ctx.run();
}};
Foo foo;
defer(ctx, cti::use_continuable)
.then([&] {
//! 1. invalid capture params in a coroutine lambda
return [](Foo foo) -> cti::continuable<> {
cout << "before: " << foo.i << "\n";
co_await timers
.emplace_back(ctx, milliseconds{10}) //! 2. timer lifetime moved out
.async_wait(cti::use_continuable);
cout << "after: " << foo.i << "\n";
co_return; //! 3. co_return added
}(foo);
})
.apply(cti::transforms::wait());
workGuard.reset();
thd.join();
return 0;
} Could you confirm that your issue is still present in this example? There are multiple points that could lead to your described behaviour:
Is it possible to modify this example to reproduce the issue again? |
When using coroutines with Asio wouldn't it be better to use Asio's built-in awaitable<> or coro<> objects ? Or have people found that the continuable<> object has less overhead ? |
In the two cases from the test code above, continuable should be completely zero overhead:
I think the awaitable from asio is zero overhead as well, so there is no advantage in using one over another from inside continuable coroutines. |
Awesome. Do you know if continuable coroutines and asio coroutines mix well ? |
For basic usage scenarios you can safely mix and match most coroutine libraries and their awaitable types. If you have any further or follow-up questions open new ticket please. |
@Spongman can you confirm that removing the edge-cases in your example fixes the mentioned issues? |
here's a more minimal repro (that triggers use-after-free):
can we get a EDIT: wait, why is that broken? that lambda isn't a coroutine.
|
@Naios
I think I have found a critical issue when awaiting nested continuables. It's a little hard to describe, but here's the code:
The issue is concerning the lifetime of the lambda object that's passed to the
then()
clause. That lambda object has aFoo
member (thefoo
capture), and that struct prints out the construction/deletion of the various copies of that member as the lambda object gets moved about.and the important part of the output is:
the local
foo
, and in fact the entire lambda object is being deleted during the handling of the nested timer callback:The text was updated successfully, but these errors were encountered: