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

Introduce spawn_detached(sender, scope, allocator) #470

Merged
merged 3 commits into from
Oct 13, 2022

Conversation

ispeters
Copy link
Contributor

This diff introduces a new algorithm, unifex::spawn_detached(). spawn_detached takes a sender, an "async scope", and an optional allocator. It nests the sender in the scope with unifex::nest, allocates an operation state using the allocator, and starts that operation. The given async scope may be anything that nest() supports, which currently includes both v1::async_scope and v2::async_scope.

This diff introduces a new algorithm, `unifex::spawn_detached()`.
`spawn_detached` takes a sender, an "async scope", and an optional
allocator.  It nests the sender in the scope with `unifex::nest`,
allocates and operation state using the allocator, and starts that
operation.  The given async scope may be anything that `nest()`
supports, which currently includes both `v1::async_scope` and
`v2::async_scope`.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 12, 2022
I had an unused capture that was breaking Clang builds.
return unifex::just();
});

try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should figure out if UNIFEX_NO_EXCEPTIONS is supported. Maybe this test should be conditional?

bool lambdaHasExecuted{false};

unifex::spawn_detached(
unifex::just_from([&, this]() noexcept {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unifex::just_from([&, this]() noexcept {
unifex::just_from([&]() noexcept {

this may fix the CI

In case we ever support `-fno-exceptions` in our CI, make the throwing
test case in `spawn_detached_test.cpp` only exist when exceptions are
enabled.
@ispeters ispeters merged commit 07ea9d4 into broken-stdlib Oct 13, 2022
@ispeters ispeters deleted the introduce_spawn_detached branch October 13, 2022 04:03
janondrusek pushed a commit to janondrusek/libunifex that referenced this pull request Apr 25, 2023
…perimental#372)

* Make async_scope::spawn return a lazy<>

This diff changes `unifex::async_scope::spawn(Sender auto)` to return a
new type, `unifex::lazy<...>`.  `lazy<>` is a _Sender_ that completes
with the result of the _Sender_ given to `spawn`.

* Fix memory leak

* Rationalize the names of some internal bits

* Fuse the promise and the operation state

This diff merges the spawned operation's promise with its operation
state so there's only one allocation.

One consequence of this change is that outstanding operations don't
record themselves as complete within their corresponding scope until the
associated `lazy<>` is either connected and started, or discarded.

* Fix build

Looks like our CI turns on exhaustive switch warnings, which I broke.

* Fix infinite hang in create_test.cpp

* Cleanup

* Last bit of cleanup before bed

* Try to fix ASAN failure

In the light of the morning, I think it's wrong to set the event before
requesting stop on the stop source because setting the event could lead
to destruction of the operation state, invalidating the stop source.

* Fix request_stop()

The callers of `request_stop()` are not owners so they may not call
`decref()`, which means I can't invoke `set_done()` from
`request_stop()`.

* Clean up, bug fixes, comments

Among other clean-up, this diff fixes a stack-use-after-return in
`future<>`'s `connect()`.

* Add constraints and run clang-format

* Comments, tests, bug fixes, and clang-format

* Restore nothrow assertion in detached_spawn_call_on

* Round out the async_scope tests

add async_scope::attach (facebookexperimental#392)

* returned `Sender` needs to be connected and started
* avoids paying penalty of `future<>`

Fix `record_done` ordering in `async_scope::attach` (facebookexperimental#424)

* `record_done`, which decrements outstanding operation count, must be called after `set_*`
* add regression test

fix cancellation race in async_scope::attach* (facebookexperimental#425)

* use refcount to pass ownership of `deliver_result`
* replace `fused_stop_source` with `inplace_stop_source`

make `async_scope::attached_sender` copyable (facebookexperimental#428)

* copy constructor calls `async_scope::try_record_start` internally
* copy of attached Sender will increment outstanding number of
  operations on async_scope

`async_scope::attach` cleanup (facebookexperimental#433)

* remove unused template argument
* add missing test case

update `async_scope` docs (facebookexperimental#434)

* spawn() -> detached_spawn()
* spawn() returns a `future`
* add missing public methods

add `async_scope::attach` docs (facebookexperimental#434)

fix `async_scope_test::attach_record_done` (facebookexperimental#437)

fix `tag_invoke(CPO)` in `async_scope` (facebookexperimental#462)

add `unifex::v2::async_scope` (facebookexperimental#463)

* simpler than `unifex::v1::async_scope` (`nest()` and `join()`)
* does not support cancellation

Introduce unifex::nest() (facebookexperimental#468)

`unifex::nest()` is a CPO that delegates to either a `tag_invoke`
customization taking a *Sender* and a "scope" reference, or to a
member function on the given scope that takes a *Sender*.  This
diff wires `unifex::nest()` to the `nest()` member function on
`v2::async_scope` and to the `attach()` member function on
`v1::async_scope`.

Introduce spawn_detached(sender, scope, allocator) (facebookexperimental#470)

This diff introduces a new algorithm, `unifex::spawn_detached()`.
`spawn_detached` takes a sender, an "async scope", and an optional
allocator.  It nests the sender in the scope with `unifex::nest`,
allocates and operation state using the allocator, and starts that
operation.  The given async scope may be anything that `nest()`
supports, which currently includes both `v1::async_scope` and
`v2::async_scope`.

Add an internal receiver to v2::async_scope's nest op (facebookexperimental#484)

While writing `unifex::spawn_future()`, I discovered that waiting until
the destructor of the `v2::async_scope`'s `nest()` operation to drop the
scope reference is too late (it led to hangs).  This diff adds an
internal receiver to the nest operation so we can detect when the
operation is complete (which is likely before the operation state is
destroyed) and drop the reference as soon as we reach that state.

Fix stop_when's handling of stop requests (facebookexperimental#500)

* Fix stop_when's handling of stop requests

When trying to sync PR facebookexperimental#495 into our internal repo, I discovered that
there's a lifetime issue in `stop_when()`.  If the `stop_when()`
operation receives a stop request from its Receiver and the
last-to-finish child operation completes synchronously in response to
the stop request then `stop_when()`'s stop callback will access the
internal stop source after it's been destroyed.

This first diff just formats `test/stop_when_test.cpp` and
`include/unifex/stop_when.hpp` with `clang-format` in preparation for
fixing the above problem.

* Add a broken unit test

This diff adds a unit test to `test/stop_when_test.cpp` that crashes
when ASAN is enabled because it dereferences a destroyed
`inplace_stop_source`.

* Increment stop_when's refcount in its stop callback

This diff fixes the broken test from the previous diff by incrementing
the `stop_when` operation's refcount while processing a stop request
from the receiver.

Add spawn_future() (facebookexperimental#489)

This diff adds `unifex::spawn_future(Sender, Scope)`.

Implement async_scope::spawn with spawn_future (facebookexperimental#501)

This diff reimplements the `v1::async_scope::spawn()` method in terms of
the newly-added `unifex::spawn_future()` algorithm.  I had to delete a
few tests that exercise behaviour that's no longer supported.

Work around an MSVC bug in C++20 mode (facebookexperimental#492)

* merge unit test that depends on `v2/async_scope`

Co-authored-by: Ian Petersen <ispeters@gmail.com>
janondrusek pushed a commit to janondrusek/libunifex that referenced this pull request Apr 25, 2023
…perimental#372)

* Make async_scope::spawn return a lazy<>

This diff changes `unifex::async_scope::spawn(Sender auto)` to return a
new type, `unifex::lazy<...>`.  `lazy<>` is a _Sender_ that completes
with the result of the _Sender_ given to `spawn`.

* Fix memory leak

* Rationalize the names of some internal bits

* Fuse the promise and the operation state

This diff merges the spawned operation's promise with its operation
state so there's only one allocation.

One consequence of this change is that outstanding operations don't
record themselves as complete within their corresponding scope until the
associated `lazy<>` is either connected and started, or discarded.

* Fix build

Looks like our CI turns on exhaustive switch warnings, which I broke.

* Fix infinite hang in create_test.cpp

* Cleanup

* Last bit of cleanup before bed

* Try to fix ASAN failure

In the light of the morning, I think it's wrong to set the event before
requesting stop on the stop source because setting the event could lead
to destruction of the operation state, invalidating the stop source.

* Fix request_stop()

The callers of `request_stop()` are not owners so they may not call
`decref()`, which means I can't invoke `set_done()` from
`request_stop()`.

* Clean up, bug fixes, comments

Among other clean-up, this diff fixes a stack-use-after-return in
`future<>`'s `connect()`.

* Add constraints and run clang-format

* Comments, tests, bug fixes, and clang-format

* Restore nothrow assertion in detached_spawn_call_on

* Round out the async_scope tests

add async_scope::attach (facebookexperimental#392)

* returned `Sender` needs to be connected and started
* avoids paying penalty of `future<>`

Fix `record_done` ordering in `async_scope::attach` (facebookexperimental#424)

* `record_done`, which decrements outstanding operation count, must be called after `set_*`
* add regression test

fix cancellation race in async_scope::attach* (facebookexperimental#425)

* use refcount to pass ownership of `deliver_result`
* replace `fused_stop_source` with `inplace_stop_source`

make `async_scope::attached_sender` copyable (facebookexperimental#428)

* copy constructor calls `async_scope::try_record_start` internally
* copy of attached Sender will increment outstanding number of
  operations on async_scope

`async_scope::attach` cleanup (facebookexperimental#433)

* remove unused template argument
* add missing test case

update `async_scope` docs (facebookexperimental#434)

* spawn() -> detached_spawn()
* spawn() returns a `future`
* add missing public methods

add `async_scope::attach` docs (facebookexperimental#434)

fix `async_scope_test::attach_record_done` (facebookexperimental#437)

fix `tag_invoke(CPO)` in `async_scope` (facebookexperimental#462)

add `unifex::v2::async_scope` (facebookexperimental#463)

* simpler than `unifex::v1::async_scope` (`nest()` and `join()`)
* does not support cancellation

Introduce unifex::nest() (facebookexperimental#468)

`unifex::nest()` is a CPO that delegates to either a `tag_invoke`
customization taking a *Sender* and a "scope" reference, or to a
member function on the given scope that takes a *Sender*.  This
diff wires `unifex::nest()` to the `nest()` member function on
`v2::async_scope` and to the `attach()` member function on
`v1::async_scope`.

Introduce spawn_detached(sender, scope, allocator) (facebookexperimental#470)

This diff introduces a new algorithm, `unifex::spawn_detached()`.
`spawn_detached` takes a sender, an "async scope", and an optional
allocator.  It nests the sender in the scope with `unifex::nest`,
allocates and operation state using the allocator, and starts that
operation.  The given async scope may be anything that `nest()`
supports, which currently includes both `v1::async_scope` and
`v2::async_scope`.

Add an internal receiver to v2::async_scope's nest op (facebookexperimental#484)

While writing `unifex::spawn_future()`, I discovered that waiting until
the destructor of the `v2::async_scope`'s `nest()` operation to drop the
scope reference is too late (it led to hangs).  This diff adds an
internal receiver to the nest operation so we can detect when the
operation is complete (which is likely before the operation state is
destroyed) and drop the reference as soon as we reach that state.

Fix stop_when's handling of stop requests (facebookexperimental#500)

* Fix stop_when's handling of stop requests

When trying to sync PR facebookexperimental#495 into our internal repo, I discovered that
there's a lifetime issue in `stop_when()`.  If the `stop_when()`
operation receives a stop request from its Receiver and the
last-to-finish child operation completes synchronously in response to
the stop request then `stop_when()`'s stop callback will access the
internal stop source after it's been destroyed.

This first diff just formats `test/stop_when_test.cpp` and
`include/unifex/stop_when.hpp` with `clang-format` in preparation for
fixing the above problem.

* Add a broken unit test

This diff adds a unit test to `test/stop_when_test.cpp` that crashes
when ASAN is enabled because it dereferences a destroyed
`inplace_stop_source`.

* Increment stop_when's refcount in its stop callback

This diff fixes the broken test from the previous diff by incrementing
the `stop_when` operation's refcount while processing a stop request
from the receiver.

Add spawn_future() (facebookexperimental#489)

This diff adds `unifex::spawn_future(Sender, Scope)`.

Implement async_scope::spawn with spawn_future (facebookexperimental#501)

This diff reimplements the `v1::async_scope::spawn()` method in terms of
the newly-added `unifex::spawn_future()` algorithm.  I had to delete a
few tests that exercise behaviour that's no longer supported.

Work around an MSVC bug in C++20 mode (facebookexperimental#492)

* merge unit test that depends on `v2/async_scope`

Co-authored-by: Ian Petersen <ispeters@gmail.com>
janondrusek pushed a commit to janondrusek/libunifex that referenced this pull request Apr 26, 2023
…perimental#372)

* Make async_scope::spawn return a lazy<>

This diff changes `unifex::async_scope::spawn(Sender auto)` to return a
new type, `unifex::lazy<...>`.  `lazy<>` is a _Sender_ that completes
with the result of the _Sender_ given to `spawn`.

* Fix memory leak

* Rationalize the names of some internal bits

* Fuse the promise and the operation state

This diff merges the spawned operation's promise with its operation
state so there's only one allocation.

One consequence of this change is that outstanding operations don't
record themselves as complete within their corresponding scope until the
associated `lazy<>` is either connected and started, or discarded.

* Fix build

Looks like our CI turns on exhaustive switch warnings, which I broke.

* Fix infinite hang in create_test.cpp

* Cleanup

* Last bit of cleanup before bed

* Try to fix ASAN failure

In the light of the morning, I think it's wrong to set the event before
requesting stop on the stop source because setting the event could lead
to destruction of the operation state, invalidating the stop source.

* Fix request_stop()

The callers of `request_stop()` are not owners so they may not call
`decref()`, which means I can't invoke `set_done()` from
`request_stop()`.

* Clean up, bug fixes, comments

Among other clean-up, this diff fixes a stack-use-after-return in
`future<>`'s `connect()`.

* Add constraints and run clang-format

* Comments, tests, bug fixes, and clang-format

* Restore nothrow assertion in detached_spawn_call_on

* Round out the async_scope tests

add async_scope::attach (facebookexperimental#392)

* returned `Sender` needs to be connected and started
* avoids paying penalty of `future<>`

Fix `record_done` ordering in `async_scope::attach` (facebookexperimental#424)

* `record_done`, which decrements outstanding operation count, must be called after `set_*`
* add regression test

fix cancellation race in async_scope::attach* (facebookexperimental#425)

* use refcount to pass ownership of `deliver_result`
* replace `fused_stop_source` with `inplace_stop_source`

make `async_scope::attached_sender` copyable (facebookexperimental#428)

* copy constructor calls `async_scope::try_record_start` internally
* copy of attached Sender will increment outstanding number of
  operations on async_scope

`async_scope::attach` cleanup (facebookexperimental#433)

* remove unused template argument
* add missing test case

update `async_scope` docs (facebookexperimental#434)

* spawn() -> detached_spawn()
* spawn() returns a `future`
* add missing public methods

add `async_scope::attach` docs (facebookexperimental#434)

fix `async_scope_test::attach_record_done` (facebookexperimental#437)

fix `tag_invoke(CPO)` in `async_scope` (facebookexperimental#462)

add `unifex::v2::async_scope` (facebookexperimental#463)

* simpler than `unifex::v1::async_scope` (`nest()` and `join()`)
* does not support cancellation

Introduce unifex::nest() (facebookexperimental#468)

`unifex::nest()` is a CPO that delegates to either a `tag_invoke`
customization taking a *Sender* and a "scope" reference, or to a
member function on the given scope that takes a *Sender*.  This
diff wires `unifex::nest()` to the `nest()` member function on
`v2::async_scope` and to the `attach()` member function on
`v1::async_scope`.

Introduce spawn_detached(sender, scope, allocator) (facebookexperimental#470)

This diff introduces a new algorithm, `unifex::spawn_detached()`.
`spawn_detached` takes a sender, an "async scope", and an optional
allocator.  It nests the sender in the scope with `unifex::nest`,
allocates and operation state using the allocator, and starts that
operation.  The given async scope may be anything that `nest()`
supports, which currently includes both `v1::async_scope` and
`v2::async_scope`.

Add an internal receiver to v2::async_scope's nest op (facebookexperimental#484)

While writing `unifex::spawn_future()`, I discovered that waiting until
the destructor of the `v2::async_scope`'s `nest()` operation to drop the
scope reference is too late (it led to hangs).  This diff adds an
internal receiver to the nest operation so we can detect when the
operation is complete (which is likely before the operation state is
destroyed) and drop the reference as soon as we reach that state.

Fix stop_when's handling of stop requests (facebookexperimental#500)

* Fix stop_when's handling of stop requests

When trying to sync PR facebookexperimental#495 into our internal repo, I discovered that
there's a lifetime issue in `stop_when()`.  If the `stop_when()`
operation receives a stop request from its Receiver and the
last-to-finish child operation completes synchronously in response to
the stop request then `stop_when()`'s stop callback will access the
internal stop source after it's been destroyed.

This first diff just formats `test/stop_when_test.cpp` and
`include/unifex/stop_when.hpp` with `clang-format` in preparation for
fixing the above problem.

* Add a broken unit test

This diff adds a unit test to `test/stop_when_test.cpp` that crashes
when ASAN is enabled because it dereferences a destroyed
`inplace_stop_source`.

* Increment stop_when's refcount in its stop callback

This diff fixes the broken test from the previous diff by incrementing
the `stop_when` operation's refcount while processing a stop request
from the receiver.

Add spawn_future() (facebookexperimental#489)

This diff adds `unifex::spawn_future(Sender, Scope)`.

Implement async_scope::spawn with spawn_future (facebookexperimental#501)

This diff reimplements the `v1::async_scope::spawn()` method in terms of
the newly-added `unifex::spawn_future()` algorithm.  I had to delete a
few tests that exercise behaviour that's no longer supported.

Work around an MSVC bug in C++20 mode (facebookexperimental#492)

* merge unit test that depends on `v2/async_scope`

Co-authored-by: Ian Petersen <ispeters@gmail.com>
janondrusek pushed a commit that referenced this pull request Apr 27, 2023
* Make async_scope::spawn return a lazy<>

This diff changes `unifex::async_scope::spawn(Sender auto)` to return a
new type, `unifex::lazy<...>`.  `lazy<>` is a _Sender_ that completes
with the result of the _Sender_ given to `spawn`.

* Fix memory leak

* Rationalize the names of some internal bits

* Fuse the promise and the operation state

This diff merges the spawned operation's promise with its operation
state so there's only one allocation.

One consequence of this change is that outstanding operations don't
record themselves as complete within their corresponding scope until the
associated `lazy<>` is either connected and started, or discarded.

* Fix build

Looks like our CI turns on exhaustive switch warnings, which I broke.

* Fix infinite hang in create_test.cpp

* Cleanup

* Last bit of cleanup before bed

* Try to fix ASAN failure

In the light of the morning, I think it's wrong to set the event before
requesting stop on the stop source because setting the event could lead
to destruction of the operation state, invalidating the stop source.

* Fix request_stop()

The callers of `request_stop()` are not owners so they may not call
`decref()`, which means I can't invoke `set_done()` from
`request_stop()`.

* Clean up, bug fixes, comments

Among other clean-up, this diff fixes a stack-use-after-return in
`future<>`'s `connect()`.

* Add constraints and run clang-format

* Comments, tests, bug fixes, and clang-format

* Restore nothrow assertion in detached_spawn_call_on

* Round out the async_scope tests

add async_scope::attach (#392)

* returned `Sender` needs to be connected and started
* avoids paying penalty of `future<>`

Fix `record_done` ordering in `async_scope::attach` (#424)

* `record_done`, which decrements outstanding operation count, must be called after `set_*`
* add regression test

fix cancellation race in async_scope::attach* (#425)

* use refcount to pass ownership of `deliver_result`
* replace `fused_stop_source` with `inplace_stop_source`

make `async_scope::attached_sender` copyable (#428)

* copy constructor calls `async_scope::try_record_start` internally
* copy of attached Sender will increment outstanding number of
  operations on async_scope

`async_scope::attach` cleanup (#433)

* remove unused template argument
* add missing test case

update `async_scope` docs (#434)

* spawn() -> detached_spawn()
* spawn() returns a `future`
* add missing public methods

add `async_scope::attach` docs (#434)

fix `async_scope_test::attach_record_done` (#437)

fix `tag_invoke(CPO)` in `async_scope` (#462)

add `unifex::v2::async_scope` (#463)

* simpler than `unifex::v1::async_scope` (`nest()` and `join()`)
* does not support cancellation

Introduce unifex::nest() (#468)

`unifex::nest()` is a CPO that delegates to either a `tag_invoke`
customization taking a *Sender* and a "scope" reference, or to a
member function on the given scope that takes a *Sender*.  This
diff wires `unifex::nest()` to the `nest()` member function on
`v2::async_scope` and to the `attach()` member function on
`v1::async_scope`.

Introduce spawn_detached(sender, scope, allocator) (#470)

This diff introduces a new algorithm, `unifex::spawn_detached()`.
`spawn_detached` takes a sender, an "async scope", and an optional
allocator.  It nests the sender in the scope with `unifex::nest`,
allocates and operation state using the allocator, and starts that
operation.  The given async scope may be anything that `nest()`
supports, which currently includes both `v1::async_scope` and
`v2::async_scope`.

Add an internal receiver to v2::async_scope's nest op (#484)

While writing `unifex::spawn_future()`, I discovered that waiting until
the destructor of the `v2::async_scope`'s `nest()` operation to drop the
scope reference is too late (it led to hangs).  This diff adds an
internal receiver to the nest operation so we can detect when the
operation is complete (which is likely before the operation state is
destroyed) and drop the reference as soon as we reach that state.

Fix stop_when's handling of stop requests (#500)

* Fix stop_when's handling of stop requests

When trying to sync PR #495 into our internal repo, I discovered that
there's a lifetime issue in `stop_when()`.  If the `stop_when()`
operation receives a stop request from its Receiver and the
last-to-finish child operation completes synchronously in response to
the stop request then `stop_when()`'s stop callback will access the
internal stop source after it's been destroyed.

This first diff just formats `test/stop_when_test.cpp` and
`include/unifex/stop_when.hpp` with `clang-format` in preparation for
fixing the above problem.

* Add a broken unit test

This diff adds a unit test to `test/stop_when_test.cpp` that crashes
when ASAN is enabled because it dereferences a destroyed
`inplace_stop_source`.

* Increment stop_when's refcount in its stop callback

This diff fixes the broken test from the previous diff by incrementing
the `stop_when` operation's refcount while processing a stop request
from the receiver.

Add spawn_future() (#489)

This diff adds `unifex::spawn_future(Sender, Scope)`.

Implement async_scope::spawn with spawn_future (#501)

This diff reimplements the `v1::async_scope::spawn()` method in terms of
the newly-added `unifex::spawn_future()` algorithm.  I had to delete a
few tests that exercise behaviour that's no longer supported.

Work around an MSVC bug in C++20 mode (#492)

* merge unit test that depends on `v2/async_scope`

Co-authored-by: Ian Petersen <ispeters@gmail.com>
hawkinsw pushed a commit to hawkinsw/libunifex that referenced this pull request May 10, 2023
…perimental#372)

* Make async_scope::spawn return a lazy<>

This diff changes `unifex::async_scope::spawn(Sender auto)` to return a
new type, `unifex::lazy<...>`.  `lazy<>` is a _Sender_ that completes
with the result of the _Sender_ given to `spawn`.

* Fix memory leak

* Rationalize the names of some internal bits

* Fuse the promise and the operation state

This diff merges the spawned operation's promise with its operation
state so there's only one allocation.

One consequence of this change is that outstanding operations don't
record themselves as complete within their corresponding scope until the
associated `lazy<>` is either connected and started, or discarded.

* Fix build

Looks like our CI turns on exhaustive switch warnings, which I broke.

* Fix infinite hang in create_test.cpp

* Cleanup

* Last bit of cleanup before bed

* Try to fix ASAN failure

In the light of the morning, I think it's wrong to set the event before
requesting stop on the stop source because setting the event could lead
to destruction of the operation state, invalidating the stop source.

* Fix request_stop()

The callers of `request_stop()` are not owners so they may not call
`decref()`, which means I can't invoke `set_done()` from
`request_stop()`.

* Clean up, bug fixes, comments

Among other clean-up, this diff fixes a stack-use-after-return in
`future<>`'s `connect()`.

* Add constraints and run clang-format

* Comments, tests, bug fixes, and clang-format

* Restore nothrow assertion in detached_spawn_call_on

* Round out the async_scope tests

add async_scope::attach (facebookexperimental#392)

* returned `Sender` needs to be connected and started
* avoids paying penalty of `future<>`

Fix `record_done` ordering in `async_scope::attach` (facebookexperimental#424)

* `record_done`, which decrements outstanding operation count, must be called after `set_*`
* add regression test

fix cancellation race in async_scope::attach* (facebookexperimental#425)

* use refcount to pass ownership of `deliver_result`
* replace `fused_stop_source` with `inplace_stop_source`

make `async_scope::attached_sender` copyable (facebookexperimental#428)

* copy constructor calls `async_scope::try_record_start` internally
* copy of attached Sender will increment outstanding number of
  operations on async_scope

`async_scope::attach` cleanup (facebookexperimental#433)

* remove unused template argument
* add missing test case

update `async_scope` docs (facebookexperimental#434)

* spawn() -> detached_spawn()
* spawn() returns a `future`
* add missing public methods

add `async_scope::attach` docs (facebookexperimental#434)

fix `async_scope_test::attach_record_done` (facebookexperimental#437)

fix `tag_invoke(CPO)` in `async_scope` (facebookexperimental#462)

add `unifex::v2::async_scope` (facebookexperimental#463)

* simpler than `unifex::v1::async_scope` (`nest()` and `join()`)
* does not support cancellation

Introduce unifex::nest() (facebookexperimental#468)

`unifex::nest()` is a CPO that delegates to either a `tag_invoke`
customization taking a *Sender* and a "scope" reference, or to a
member function on the given scope that takes a *Sender*.  This
diff wires `unifex::nest()` to the `nest()` member function on
`v2::async_scope` and to the `attach()` member function on
`v1::async_scope`.

Introduce spawn_detached(sender, scope, allocator) (facebookexperimental#470)

This diff introduces a new algorithm, `unifex::spawn_detached()`.
`spawn_detached` takes a sender, an "async scope", and an optional
allocator.  It nests the sender in the scope with `unifex::nest`,
allocates and operation state using the allocator, and starts that
operation.  The given async scope may be anything that `nest()`
supports, which currently includes both `v1::async_scope` and
`v2::async_scope`.

Add an internal receiver to v2::async_scope's nest op (facebookexperimental#484)

While writing `unifex::spawn_future()`, I discovered that waiting until
the destructor of the `v2::async_scope`'s `nest()` operation to drop the
scope reference is too late (it led to hangs).  This diff adds an
internal receiver to the nest operation so we can detect when the
operation is complete (which is likely before the operation state is
destroyed) and drop the reference as soon as we reach that state.

Fix stop_when's handling of stop requests (facebookexperimental#500)

* Fix stop_when's handling of stop requests

When trying to sync PR facebookexperimental#495 into our internal repo, I discovered that
there's a lifetime issue in `stop_when()`.  If the `stop_when()`
operation receives a stop request from its Receiver and the
last-to-finish child operation completes synchronously in response to
the stop request then `stop_when()`'s stop callback will access the
internal stop source after it's been destroyed.

This first diff just formats `test/stop_when_test.cpp` and
`include/unifex/stop_when.hpp` with `clang-format` in preparation for
fixing the above problem.

* Add a broken unit test

This diff adds a unit test to `test/stop_when_test.cpp` that crashes
when ASAN is enabled because it dereferences a destroyed
`inplace_stop_source`.

* Increment stop_when's refcount in its stop callback

This diff fixes the broken test from the previous diff by incrementing
the `stop_when` operation's refcount while processing a stop request
from the receiver.

Add spawn_future() (facebookexperimental#489)

This diff adds `unifex::spawn_future(Sender, Scope)`.

Implement async_scope::spawn with spawn_future (facebookexperimental#501)

This diff reimplements the `v1::async_scope::spawn()` method in terms of
the newly-added `unifex::spawn_future()` algorithm.  I had to delete a
few tests that exercise behaviour that's no longer supported.

Work around an MSVC bug in C++20 mode (facebookexperimental#492)

* merge unit test that depends on `v2/async_scope`

Co-authored-by: Ian Petersen <ispeters@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants