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

Deprecate and remove 'eager' task classes task and shared_task #29

Closed
lewissbaker opened this issue Aug 12, 2017 · 5 comments
Closed

Deprecate and remove 'eager' task classes task and shared_task #29

lewissbaker opened this issue Aug 12, 2017 · 5 comments
Assignees

Comments

@lewissbaker
Copy link
Owner

lewissbaker commented Aug 12, 2017

Once we have a way to synchronously block waiting for a task in #27 and when_all in #10 there shouldn't be any need for the eagerly started task types any more and we can limit use to just the lazy task types (ie. lazy_task and shared_lazy_task)

One of the big motivations for getting rid of eagerly-started tasks is that it is difficult to write exception-safe code with use of eagerly-started tasks that aren't immediately co_awaited.

We don't want to leave dangling tasks/computation as this can lead to ignored/uncaught exceptions or unsynchronised access to shared resources.

For example, consider the case where we want to execute two tasks concurrently and wait for them both to finish before continuing, using their results:

task<A> do1();
task<B> do2();

task<> do1and2concurrently_unsafe()
{
  // The calls to do1() and do2() start the tasks eagerly but are potentially executed
  // in any order and either one of them could fail after the other has already started
  // a concurrent computation.
  auto [a, b] = cppcoro::when_all(do1(), do2());

  // use a and b ...
}

To implement this in an exception-safe way we'd need to modify the function as follows:

task<> do1and2concurrently_safe()
{
  // This implicitly starts executing the task and it executes concurrently with the
  // rest of the logic below until we co_await the task.
  task<A> t1 = do1();

  // Now we need to start the do2() task but since this could fail we need
  // to start it inside a try/catch so we can wait for t1 to finish before it
  // goes out of scope (we don't want to leave any dangling computation).
  task<B> t2;
  std::exception_ptr ex;
  try
  {
    // This call might throw, since it may need to allocate a coroutine frame
    // which could fail with std::bad_alloc.
    t2 = do2();
  }
  catch (...)
  {
    // Don't want to leave t1 still executing but we can't co_await inside catch-block
    // So we capture current exception and do co_await outside catch block.
    ex = std::current_exception();
  }

  if (ex)
  {
    // Wait until t1 completes before rethrowing the exception.
    co_await t1.when_ready();
    std::rethrow_exception(ex);
  }

  // Now that we have both t1 and t2 started successfully we can use when_all() to get the results.
  auto [a, b] = when_all(std::move(t1), std::move(t2));

  // use a, b ...  
}

Compared with lazy_task version which is both concise and exception-safe:

lazy_task<> do1();
lazy_task<> do2();

lazy_task<> do1and2concurrently()
{
  // The calls to do1() and do2() can still execute in any order but all they
  // do is allocate coroutine frames, they don't start any computation.
  // If the second call fails then the normal stack-unwinding will ensure the
  // first coroutine frame is destroyed. It doesn't need to worry about waiting
  // for the first task to complete. Either they both start or none of them do.
  auto [a, b] = cppcoro::when_all(do1(), do2());

  // use a and b ...
}

With a lazy_task, the task is either being co_awaited by some other coroutine or it is not executing (it has either not yet started, or has completed executing) and so the lazy_task is always safe to destruct and will free the coroutine frame it owns.

A side-benefit of using lazy_task everywhere is that it can be implemented without the need for std::atomic operations to synchronise coroutine completion and awaiter. This can have some potential benefits for performance by avoiding use of atomic operations for basic sequential flow of execution.

@lewissbaker
Copy link
Owner Author

Started the process of removing use of task<T> from tests in b78bc3e.

There are still the where_all_tests.cpp and where_all_ready_tests.cpp to go but I've been running into some crashes when updating these that I need to investigate.

@lewissbaker
Copy link
Owner Author

Commit 722fe1b removes use of eager tasks from where_all_ready_tests.cpp.

Only where_all_tests.cpp to go.

@lewissbaker lewissbaker self-assigned this Aug 15, 2017
@lewissbaker
Copy link
Owner Author

lewissbaker commented Aug 15, 2017

Commit 421de76 removes last of eager task usage from tests (except where testing eager tasks themselves).

@lewissbaker
Copy link
Owner Author

Commit 64b0a04 removes the task<T> and shared_task<T> classes and updates the README.

All that remains now is renaming lazy_task -> task and shared_lazy_task -> shared_task.

@lewissbaker
Copy link
Owner Author

Commit bf9eb2c has implemented the rename.

Closing.

Garcia6l20 pushed a commit to Garcia6l20/cppcoro that referenced this issue Jan 5, 2021
…pveyor

Remove cake, appveyor and unmaintained scripts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant