-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Introduce notified_owned
for thread-safe notification futures.
#7391
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
Conversation
Allows returning an owned notification future that can be moved across threads. Adds `NotifyRef` enum to manage borrowed and owned references internally.
Allows returning an owned notification future that can be moved across threads. Adds `NotifyRef` enum to manage borrowed and owned references internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[derive(Debug)] | ||
enum NotifyRef<'a> { | ||
Owned(Arc<Notify>), | ||
Ref(&'a Notify), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we want more future types, but if we do, it has to be a new type. An enum pays a cost for everyone using notified
, and we don't want to go that route.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, I didn't take into account the cost of the access, any workaround for not repeating the implementation between Notified and NotifiedOwned ? I suppose using an union is cursed, maybe a macro ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid unions or macros. How about extracting common logic into shared functions?
Hi, any update on this ? if you’re no longer working on it, id be happy to take it over. |
Sorry, you can take over. I don't have time right now to work on it
Le ven. 18 juil. 2025, 13:44, Aria Andika ***@***.***> a
écrit :
… *ariaandika* left a comment (tokio-rs/tokio#7391)
<#7391 (comment)>
Hi, any update on this ? if you’re no longer working on it, id be happy to
take it over.
—
Reply to this email directly, view it on GitHub
<#7391 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACQOJGDT6TW66SDQUKG3QJ33JDM3BAVCNFSM6AAAAAB6XSFJGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAOBZGIYDGMJSHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Allows returning an owned notification future that can be moved across threads. Inspired by #7231
Motivation
A common pattern when using Notify is to launch tasks and then notify them at the same time using
notify_waiters
. In the current API it can be quite challenging because you don't know if the task got the time to register before notifying. A good way of doing it would be to register then launch the task. Problem to do that you would need the result future fromnotified()
(akaNotified<'_>
) to be Send.Ex:
Ex: Workaround using a oneshot and Shared<_>
Solution
Added a new method
notified_owned
that returnsSend
Notified<'static>
.