Skip to content

Commit

Permalink
Solve review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@DJMcNab
DJMcNab committed Aug 29, 2023
1 parent d618a61 commit 831ddaa
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 26 deletions.
24 changes: 12 additions & 12 deletions src/loop_logic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub struct RegistrationToken {
pub(crate) struct LoopInner<'l, Data> {
pub(crate) poll: RefCell<Poll>,
pub(crate) sources: RefCell<Slab<Rc<dyn EventDispatcher<Data> + 'l>>>,
pub(crate) sources_with_additional_lifetime_events: AdditionalLifetimeEventsSet,
pub(crate) sources_with_additional_lifecycle_events: AdditionalLifetimeEventsSet,
idles: RefCell<Vec<IdleCallback<'l, Data>>>,
pending_action: Cell<PostAction>,
}
Expand Down Expand Up @@ -141,7 +141,7 @@ impl<'l, Data> LoopHandle<'l, Data> {
let ret = sources.get(key).unwrap().register(
&mut poll,
self.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.create_register_for_token(RegistrationToken { key }),
&mut TokenFactory::new(key),
);
Expand Down Expand Up @@ -179,7 +179,7 @@ impl<'l, Data> LoopHandle<'l, Data> {
source.register(
&mut self.inner.poll.borrow_mut(),
self.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.create_register_for_token(*token),
&mut TokenFactory::new(token.key),
)?;
Expand All @@ -196,7 +196,7 @@ impl<'l, Data> LoopHandle<'l, Data> {
if !source.reregister(
&mut self.inner.poll.borrow_mut(),
self.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.create_register_for_token(*token),
&mut TokenFactory::new(token.key),
)? {
Expand All @@ -215,7 +215,7 @@ impl<'l, Data> LoopHandle<'l, Data> {
if !source.unregister(
&mut self.inner.poll.borrow_mut(),
self.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.create_register_for_token(*token),
)? {
// we are in a callback, store for later processing
Expand All @@ -231,7 +231,7 @@ impl<'l, Data> LoopHandle<'l, Data> {
if let Err(e) = source.unregister(
&mut self.inner.poll.borrow_mut(),
self.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.create_register_for_token(token),
) {
log::warn!(
Expand Down Expand Up @@ -293,7 +293,7 @@ impl<'l, Data> EventLoop<'l, Data> {
sources: RefCell::new(Slab::new()),
idles: RefCell::new(Vec::new()),
pending_action: Cell::new(PostAction::Continue),
sources_with_additional_lifetime_events: Default::default(),
sources_with_additional_lifecycle_events: Default::default(),
}),
};

Expand Down Expand Up @@ -323,7 +323,7 @@ impl<'l, Data> EventLoop<'l, Data> {
let mut extra_lifecycle_sources = self
.handle
.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.values
.borrow_mut();
let sources = &self.handle.inner.sources.borrow();
Expand Down Expand Up @@ -366,7 +366,7 @@ impl<'l, Data> EventLoop<'l, Data> {
let mut extra_lifecycle_sources = self
.handle
.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.values
.borrow_mut();
if !extra_lifecycle_sources.is_empty() {
Expand Down Expand Up @@ -416,7 +416,7 @@ impl<'l, Data> EventLoop<'l, Data> {
&mut self.handle.inner.poll.borrow_mut(),
self.handle
.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.create_register_for_token(RegistrationToken {
key: registroken_token,
}),
Expand All @@ -428,7 +428,7 @@ impl<'l, Data> EventLoop<'l, Data> {
&mut self.handle.inner.poll.borrow_mut(),
self.handle
.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.create_register_for_token(RegistrationToken {
key: registroken_token,
}),
Expand Down Expand Up @@ -458,7 +458,7 @@ impl<'l, Data> EventLoop<'l, Data> {
&mut poll,
self.handle
.inner
.sources_with_additional_lifetime_events
.sources_with_additional_lifecycle_events
.create_register_for_token(RegistrationToken {
key: registroken_token,
}),
Expand Down
35 changes: 21 additions & 14 deletions src/sources/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ impl<T: EventSource> EventSource for &mut T {
pub(crate) struct DispatcherInner<S, F> {
source: S,
callback: F,
needs_additional_lifetime_events: bool,
needs_additional_lifecycle_events: bool,
}

impl<Data, S, F> EventDispatcher<Data> for RefCell<DispatcherInner<S, F>>
Expand Down Expand Up @@ -314,27 +314,27 @@ where
fn register(
&self,
poll: &mut Poll,
mut additional_lifetime_register: AdditionalLifetimeEventsRegister<'_>,
mut additional_lifecycle_register: AdditionalLifetimeEventsRegister<'_>,
token_factory: &mut TokenFactory,
) -> crate::Result<()> {
let mut this = self.borrow_mut();

if this.needs_additional_lifetime_events {
additional_lifetime_register.register();
if this.needs_additional_lifecycle_events {
additional_lifecycle_register.register();
}
this.source.register(poll, token_factory)
}

fn reregister(
&self,
poll: &mut Poll,
mut additional_lifetime_register: AdditionalLifetimeEventsRegister<'_>,
mut additional_lifecycle_register: AdditionalLifetimeEventsRegister<'_>,
token_factory: &mut TokenFactory,
) -> crate::Result<bool> {
if let Ok(mut me) = self.try_borrow_mut() {
me.source.reregister(poll, token_factory)?;
if me.needs_additional_lifetime_events {
additional_lifetime_register.register();
if me.needs_additional_lifecycle_events {
additional_lifecycle_register.register();

Check warning on line 337 in src/sources/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/mod.rs#L337 

Added line #L337 was not covered by tests
}
Ok(true)
} else {
Expand All @@ -345,12 +345,12 @@ where
fn unregister(
&self,
poll: &mut Poll,
mut additional_lifetime_register: AdditionalLifetimeEventsRegister<'_>,
mut additional_lifecycle_register: AdditionalLifetimeEventsRegister<'_>,
) -> crate::Result<bool> {
if let Ok(mut me) = self.try_borrow_mut() {
me.source.unregister(poll)?;
if me.needs_additional_lifetime_events {
additional_lifetime_register.unregister();
if me.needs_additional_lifecycle_events {
additional_lifecycle_register.unregister();
}
Ok(true)
} else {
Expand Down Expand Up @@ -382,29 +382,33 @@ pub(crate) trait EventDispatcher<Data> {
fn register(
&self,
poll: &mut Poll,
additional_lifetime_register: AdditionalLifetimeEventsRegister<'_>,
additional_lifecycle_register: AdditionalLifetimeEventsRegister<'_>,
token_factory: &mut TokenFactory,
) -> crate::Result<()>;

fn reregister(
&self,
poll: &mut Poll,
additional_lifetime_register: AdditionalLifetimeEventsRegister<'_>,
additional_lifecycle_register: AdditionalLifetimeEventsRegister<'_>,
token_factory: &mut TokenFactory,
) -> crate::Result<bool>;

fn unregister(
&self,
poll: &mut Poll,
additional_lifetime_register: AdditionalLifetimeEventsRegister<'_>,
additional_lifecycle_register: AdditionalLifetimeEventsRegister<'_>,
) -> crate::Result<bool>;

fn before_sleep(&self) -> crate::Result<Option<(Readiness, Token)>>;
fn before_handle_events(&self, was_awoken: bool);
}

#[derive(Default)]
/// The list of events
pub(crate) struct AdditionalLifetimeEventsSet {
/// The list of events. The boolean indicates whether the source had an event
/// - this is stored in this list because we need to know this value for each
/// item at once - that is, to avoid allocating in the hot loop
pub(crate) values: RefCell<Vec<(RegistrationToken, bool)>>,
}

Expand All @@ -417,6 +421,9 @@ impl AdditionalLifetimeEventsSet {
}
}

/// To ensure that registering the events which need additional lifecycle events
/// happens correctly, we do this within the implementations of `EventDispatcher`
/// `AdditionalLifetimeEventsRegister` is a helper type to centralise this logic
pub(crate) struct AdditionalLifetimeEventsRegister<'a> {
set: &'a AdditionalLifetimeEventsSet,
token: RegistrationToken,
Expand Down Expand Up @@ -497,7 +504,7 @@ where
Dispatcher(Rc::new(RefCell::new(DispatcherInner {
source,
callback,
needs_additional_lifetime_events: S::NEEDS_EXTRA_LIFECYCLE_EVENTS,
needs_additional_lifecycle_events: S::NEEDS_EXTRA_LIFECYCLE_EVENTS,
})))
}

Expand Down

0 comments on commit 831ddaa

Please sign in to comment.