Comments (6)
event-listener is not used by any other smol crates public API, so I think it is okay to make a new breaking release (4.0) right now if needed.
from event-listener.
I agree with the idea that new()
should take no parameters and listen()
should take the &Event
parameter; it makes more sense that way. Unfortunately that would be breaking change as this point, and deprecation/introduction of new methods introduces its own footguns, as mixing the two types of methods would lead to unexpected panics. However, I disagree with the second option, as it would make polling/waiting on the EventListener
much more tricky.
If the listener is linked into the list on the first poll of the Future
, it makes it more difficult to receive events. Until the listener is fully linked into the list, it will not receive events. Meaning, if an event occurs while the listener is being atomically linked into the list, it will not receive that event. This makes it necessary to check any atomic flags before and after the list is inserted. This could would become necessary:
loop {
if check_the_flag() {
break;
}
let mut listener = EventListener::new(&event);
pin!(listener);
// Poll the future once to insert it into the list.
future::poll_once(listener.as_mut()).await;
if check_the_flags() {
break;
}
// Now we can fully await on the listener.
listener.await;
}
This introduces a much larger footgun, however, as a naive user may just try to .await
on the future without making sure the condition that they're waiting on hasn't changed after the insert. This code will deadlock unpredictably:
loop {
if check_the_flag() {
break;
}
let listener = EventListener::new(&event);
pin!(listener);
listener.await;
}
Inserting on first poll also neglects the wait()
, use case, which doesn't really have a "poll" mechanism at all. So the above deadlock would become the mandatory way of using wait()
, unless you do wait_timeout(Duration::from_secs(0))
or something to "register" it first. But that adds its own additional complexity.
One possible way around this is to have the first await
or wait()
on the EventListener
insert itself into the list and succeed immediately, so that the user can check the flag before await
ing/wait()
ing on the listener a second time, which would then wait properly. But this has its own nuances/drawbacks and should be discussed further if this is the direction that we want to go.
from event-listener.
Thanks so much for explaining in detail.
One possible way around this is to have the first
await
orwait()
on theEventListener
insert itself into the list and succeed immediately, so that the user can check the flag beforeawait
ing/wait()
ing on the listener a second time, which would then wait properly. But this has its own nuances/drawbacks and should be discussed further if this is the direction that we want to go.
That doesn't seem extremely bad to me but yeah, I agree it's still a bit weird/unexpected behavior. As I wrote on the Matrix channel, I've no objection on doing a 4.0 release that fixes it. If we do that, IMO we should yank the 3.0 on crates.io.
from event-listener.
Sounds good. But we should probably wait until we roll out the rest of smol
's breaking changes before then.
from event-listener.
But we should probably wait until we roll out the rest of
smol
's breaking changes before then.
That would delay zbus 4.0 even further than it already is. :( Why don't we do both:
- Deprecate
new
for another method for now (since there will be no args, I think we can just go forDefault::default
impl). - In the next break, remove the
new
method. Alternatively, we can keep new but just remove its argument.
from event-listener.
event-listener is not used by any other smol crates public API, so I think it is okay to make a new breaking release right now if needed.
Oh, even better!
from event-listener.
Related Issues (20)
- Unsoundness HOT 11
- RFC: Less complex, footgun free API HOT 2
- expose listener count HOT 4
- Remove concurrent-queue dependency
- `NonBlocking` doesn't implement `Send` and `Sync` HOT 2
- Possible racecondition HOT 1
- Question about new API HOT 3
- Miri reports a deadlock in `async-lock` HOT 2
- Upgrading v4 to v5 panic: Listener was never inserted into the list HOT 6
- RUSTSEC-2021-0145: Potential unaligned read HOT 1
- missing_docs warning in easy_wrapper implementation HOT 2
- UB due to lack of a full fence in full_fence (on x86) HOT 5
- no_std impl tries to collect std::usize::MAX flags when notified
- fmt::Debug should produce actually useful output
- Split event-listener-strategy out into a new crate HOT 2
- Miri test failure with --no-default-features HOT 1
- EventListener::new footgun HOT 1
- `EventListener` doesn't implement `UnwindSafe` HOT 1
- wasm test failed with "index out of bounds: the len is 0 but the index is 4" HOT 6
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from event-listener.