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

abort_source: modify subscribe method #2264

Closed
wants to merge 1 commit into from

Conversation

Deexie
Copy link
Contributor

@Deexie Deexie commented May 22, 2024

Currently when subscribe method is called on an abort_source on which abort has already been requested, method's arguments are ignored and std::nullopt is returned. So, behaviour of the method depends on timing and aforementioned case needs to be handled by a caller.

If subscribe is called on already aborted abort source, trigger a function passed as an argument.

Currently when subscribe method is called on an abort_source
on which abort has already been requested, method's arguments
are ignored and std::nullopt is returned. So, behaviour of
the method depends on timing and aforementioned case needs
to be handled by a caller.

If subscribe is called on already aborted abort source,
trigger a function passed as an argument.
@Deexie Deexie requested review from denesb and bhalevy May 22, 2024 14:35
@Deexie Deexie marked this pull request as draft May 22, 2024 14:42
template <typename Func>
requires (std::is_nothrow_invocable_r_v<void, Func, const std::optional<std::exception_ptr>&> ||
std::is_nothrow_invocable_r_v<void, Func>)
[[nodiscard]]
optimized_optional<subscription> subscribe(Func&& f) {
if (abort_requested()) {
f(_ex);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure can make such a semantic change without auditing all call sites.
Are all callers ready to be called back at this early stage?
In some cases this is done in constructors and there could be more initialization taking place after subscribing (even as member initialization)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see, like capturing the object while it isn't fully initialized yet, since constructor is synchronous anyway.
I missed that and the call seemed more intuitive. Closing then.

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

Successfully merging this pull request may close these issues.

None yet

2 participants