From c326d5b07ee841b75ab3c9b5223417b5f64e7e4e Mon Sep 17 00:00:00 2001 From: lumi Date: Sat, 27 May 2017 16:56:44 +0200 Subject: [PATCH] fix up the event system, no more unsafe! --- src/client.rs | 4 +- src/event.rs | 190 ++++++++++++++++----------------------- src/lib.rs | 1 - src/plugin.rs | 9 +- src/plugin_macro.rs | 12 +-- src/plugins/messaging.rs | 16 ++-- src/plugins/ping.rs | 14 ++- src/plugins/stanza.rs | 14 ++- 8 files changed, 110 insertions(+), 150 deletions(-) diff --git a/src/client.rs b/src/client.rs index 70741b88..1ce9e21d 100644 --- a/src/client.rs +++ b/src/client.rs @@ -88,11 +88,11 @@ impl ClientBuilder { binding: PluginProxyBinding::new(dispatcher.clone()), dispatcher: dispatcher, }; - client.dispatcher.lock().unwrap().register(Priority::Default, Box::new(move |evt: &SendElement| { + client.dispatcher.lock().unwrap().register(Priority::Default, move |evt: &SendElement| { let mut t = transport.lock().unwrap(); t.write_element(&evt.0).unwrap(); Propagation::Continue - })); + }); client.connect(credentials)?; client.bind()?; Ok(client) diff --git a/src/event.rs b/src/event.rs index 7841ee39..b9adebcd 100644 --- a/src/event.rs +++ b/src/event.rs @@ -3,20 +3,41 @@ use std::any::{TypeId, Any}; use std::fmt::Debug; use std::collections::BTreeMap; use std::cmp::Ordering; -use std::sync::Arc; use std::mem; -use std::ptr; -use std::raw::TraitObject; use minidom::Element; /// A marker trait which marks all events. pub trait Event: Any + Debug {} -/// A trait which can be implemented when something can handle a specific kind of event. -pub trait EventHandler: Any { +/// A trait which is implemented for all event handlers. +trait EventHandler: Any { /// Handle an event, returns whether to propagate the event to the remaining handlers. - fn handle(&self, event: &E) -> Propagation; + fn handle(&self, event: &AbstractEvent) -> Propagation; +} + +/// An abstract event. +pub struct AbstractEvent { + inner: Box, +} + +impl AbstractEvent { + /// Creates an abstract event from a concrete event. + pub fn new(event: E) -> AbstractEvent { + AbstractEvent { + inner: Box::new(event), + } + } + + /// Downcasts this abstract event into a concrete event. + pub fn downcast(&self) -> Option<&E> { + self.inner.downcast_ref::() + } + + /// Checks whether this abstract event is a specific concrete event. + pub fn is(&self) -> bool { + self.inner.is::() + } } struct Record(P, T); @@ -49,21 +70,10 @@ pub enum Propagation { Continue, } -#[derive(Debug)] -struct GarbageEvent; - -impl Event for GarbageEvent {} - -impl EventHandler for Box where E: Event, F: 'static + Fn(&E) -> Propagation { - fn handle(&self, evt: &E) -> Propagation { - self(evt) - } -} - /// An event dispatcher, this takes care of dispatching events to their respective handlers. pub struct Dispatcher { - handlers: BTreeMap>>>, - queue: Vec<(TypeId, Box)>, + handlers: BTreeMap>>>, + queue: Vec<(TypeId, AbstractEvent)>, } impl Dispatcher { @@ -76,17 +86,39 @@ impl Dispatcher { } /// Register an event handler. - pub fn register(&mut self, priority: Priority, handler: H) where E: Event + 'static, H: EventHandler { - let handler: Box> = Box::new(handler) as Box>; + pub fn register(&mut self, priority: Priority, func: F) + where + E: Event, + F: Fn(&E) -> Propagation + 'static { + struct Handler where E: Event, F: Fn(&E) -> Propagation { + func: F, + _marker: PhantomData, + } + + impl Propagation + 'static> EventHandler for Handler { + fn handle(&self, evt: &AbstractEvent) -> Propagation { + if let Some(e) = evt.downcast::() { + (self.func)(e) + } + else { + Propagation::Continue + } + } + } + + let handler: Box = Box::new(Handler { + func: func, + _marker: PhantomData, + }) as Box; let ent = self.handlers.entry(TypeId::of::()) .or_insert_with(|| Vec::new()); - ent.push(Record(priority, Box::new(handler) as Box)); + ent.push(Record(priority, handler)); ent.sort(); } /// Append an event to the queue. pub fn dispatch(&mut self, event: E) where E: Event { - self.queue.push((TypeId::of::(), Box::new(event) as Box)); + self.queue.push((TypeId::of::(), AbstractEvent::new(event))); } /// Flush all events in the queue so they can be handled by their respective handlers. @@ -97,19 +129,7 @@ impl Dispatcher { 'evts: for (t, evt) in q { if let Some(handlers) = self.handlers.get_mut(&t) { for &mut Record(_, ref mut handler) in handlers { - // GarbageEvent is a garbage type. - // The actual passed type is NEVER of this type. - let h: &mut EventHandler = unsafe { - let handler_obj: &mut TraitObject = mem::transmute(handler); - let handler_inner: *mut TraitObject = mem::transmute(handler_obj.data); - mem::transmute(*handler_inner) - }; - let e: &&GarbageEvent = unsafe { - let evt_ref: &Any = &evt; - let evt_obj: TraitObject = mem::transmute(evt_ref); - mem::transmute(evt_obj.data) - }; - match h.handle(e) { + match handler.handle(&evt) { Propagation::Stop => { continue 'evts; }, Propagation::Continue => (), } @@ -124,53 +144,6 @@ impl Dispatcher { pub fn flush_all(&mut self) { while self.flush() {} } - - /// Dispatch an event to the handlers right now, without going through the queue. - pub fn dispatch_now(&mut self, event: E) where E: Event { - if let Some(handlers) = self.handlers.get_mut(&TypeId::of::()) { - for &mut Record(_, ref mut handler) in handlers { - let h = handler.downcast_mut::>>().unwrap(); - match h.handle(&event) { - Propagation::Stop => { return; }, - Propagation::Continue => (), - } - } - } - } -} - -pub struct EventProxy { - inner: Arc>, - vtable: *mut (), - _event_type: PhantomData, -} - -impl EventProxy { - /// Unsafe because T is assumed to be a TraitObject or at least have its shape. - /// If it is not, things will break. In a fascinatingly horrible manner. - /// Some people, such as myself, find it hilarious. Most people do not. - /// T is also assumed to actually support EventHandler, if it does not, refer to above - /// statement. - pub unsafe fn new>(inner: Arc>) -> EventProxy { - let box_with_vtable = &*ptr::null::() as &EventHandler; - let obj: TraitObject = mem::transmute(box_with_vtable); - EventProxy { - inner: inner, - vtable: obj.vtable, - _event_type: PhantomData, - } - } -} - -impl EventHandler for EventProxy where Box: 'static { - fn handle(&self, evt: &E) -> Propagation { - let inner = Arc::into_raw(self.inner.clone()); - let obj = TraitObject { data: unsafe { mem::transmute(inner) }, vtable: self.vtable }; - let handler: &EventHandler = unsafe { mem::transmute(obj) }; - let prop = handler.handle(evt); - unsafe { Arc::>::from_raw(mem::transmute(inner)); } - prop - } } #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -205,10 +178,6 @@ mod tests { fn test() { let mut disp = Dispatcher::new(); - struct MyHandler; - struct EvilHandler; - struct EventFilter; - #[derive(Debug)] struct MyEvent { should_be_42: u32, @@ -216,38 +185,31 @@ mod tests { impl Event for MyEvent {} - impl EventHandler for MyHandler { - fn handle(&self, evt: &MyEvent) -> Propagation { - if evt.should_be_42 == 42 { - panic!("success"); - } - else { - panic!("not 42"); - } + disp.register(Priority::Max, |evt: &MyEvent| { + if evt.should_be_42 == 42 { + Propagation::Continue } - } - - impl EventHandler for EvilHandler { - fn handle(&self, _: &MyEvent) -> Propagation { - panic!("should not be called"); + else { + Propagation::Stop } - } + }); - impl EventHandler for EventFilter { - fn handle(&self, evt: &MyEvent) -> Propagation { - if evt.should_be_42 == 42 { - Propagation::Continue - } - else { - Propagation::Stop - } + disp.register(Priority::Min, |_: &MyEvent| { + panic!("should not be called"); + }); + + disp.register(Priority::Default, |evt: &MyEvent| { + if evt.should_be_42 == 42 { + panic!("success"); } - } + else { + panic!("not 42"); + } + }); - disp.register(Priority::Max, EventFilter); - disp.register(Priority::Min, EvilHandler); - disp.register(Priority::Default, MyHandler); - disp.register(Priority::Min, EvilHandler); + disp.register(Priority::Min, |_: &MyEvent| { + panic!("should not be called"); + }); disp.dispatch(MyEvent { should_be_42: 39, diff --git a/src/lib.rs b/src/lib.rs index 5bc9298b..9ce13a70 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,3 @@ -#![feature(raw)] #![feature(try_from)] extern crate xml; diff --git a/src/plugin.rs b/src/plugin.rs index 2ccb4606..d025edb2 100644 --- a/src/plugin.rs +++ b/src/plugin.rs @@ -1,6 +1,6 @@ //! Provides the plugin infrastructure. -use event::{Event, EventHandler, Dispatcher, SendElement, Priority}; +use event::{Event, Dispatcher, SendElement, Priority, Propagation}; use std::any::Any; @@ -62,10 +62,13 @@ impl PluginProxy { } /// Registers an event handler. - pub fn register_handler(&self, priority: Priority, handler: H) where E: Event, H: EventHandler { + pub fn register_handler(&self, priority: Priority, func: F) + where + E: Event, + F: Fn(&E) -> Propagation + 'static { self.with_binding(move |binding| { // TODO: proper error handling - binding.dispatcher.lock().unwrap().register(priority, handler); + binding.dispatcher.lock().unwrap().register(priority, func); }); } diff --git a/src/plugin_macro.rs b/src/plugin_macro.rs index 36599cce..687e2c4e 100644 --- a/src/plugin_macro.rs +++ b/src/plugin_macro.rs @@ -1,6 +1,6 @@ #[macro_export] macro_rules! impl_plugin { - ($plugin:ty, $proxy:ident, [$($evt:ty => $pri:expr),*]) => { + ($plugin:ty, $proxy:ident, [$(($evt:ty, $pri:expr) => $func:ident),*]) => { impl $crate::plugin::Plugin for $plugin { fn get_proxy(&mut self) -> &mut $crate::plugin::PluginProxy { &mut self.$proxy @@ -12,15 +12,17 @@ macro_rules! impl_plugin { fn init( dispatcher: &mut $crate::event::Dispatcher , me: ::std::sync::Arc>) { $( - dispatcher.register($pri, unsafe { - $crate::event::EventProxy::new::<$plugin>(me.clone()) + let new_arc = me.clone(); + dispatcher.register($pri, move |e: &$evt| { + let p = new_arc.as_any().downcast_ref::<$plugin>().unwrap(); + p . $func(e) }); )* } } }; - ($plugin:ty, $proxy:ident, [$($evt:ty => $pri:expr),*,]) => { - impl_plugin!($plugin, $proxy, [$($evt => $pri),*]); + ($plugin:ty, $proxy:ident, [$(($evt:ty, $pri:expr) => $func:ident),*,]) => { + impl_plugin!($plugin, $proxy, [$(($evt, $pri) => $func),*]); }; } diff --git a/src/plugins/messaging.rs b/src/plugins/messaging.rs index 1fe9d541..519d1507 100644 --- a/src/plugins/messaging.rs +++ b/src/plugins/messaging.rs @@ -1,5 +1,5 @@ -use plugin::{PluginProxy}; -use event::{Event, EventHandler, ReceiveElement, Priority, Propagation}; +use plugin::PluginProxy; +use event::{Event, ReceiveElement, Priority, Propagation}; use minidom::Element; use error::Error; use jid::Jid; @@ -34,14 +34,8 @@ impl MessagingPlugin { self.proxy.send(elem); Ok(()) } -} -impl_plugin!(MessagingPlugin, proxy, [ - ReceiveElement => Priority::Default, -]); - -impl EventHandler for MessagingPlugin { - fn handle(&self, evt: &ReceiveElement) -> Propagation { + fn handle_receive_element(&self, evt: &ReceiveElement) -> Propagation { let elem = &evt.0; if elem.is("message", ns::CLIENT) && elem.attr("type") == Some("chat") { if let Some(body) = elem.get_child("body", ns::CLIENT) { @@ -55,3 +49,7 @@ impl EventHandler for MessagingPlugin { Propagation::Continue } } + +impl_plugin!(MessagingPlugin, proxy, [ + (ReceiveElement, Priority::Default) => handle_receive_element, +]); diff --git a/src/plugins/ping.rs b/src/plugins/ping.rs index 3421d7d3..6fffe8b9 100644 --- a/src/plugins/ping.rs +++ b/src/plugins/ping.rs @@ -1,5 +1,5 @@ use plugin::PluginProxy; -use event::{Event, EventHandler, Priority, Propagation, ReceiveElement}; +use event::{Event, Priority, Propagation, ReceiveElement}; use minidom::Element; use error::Error; use jid::Jid; @@ -43,14 +43,8 @@ impl PingPlugin { .build(); self.proxy.send(reply); } -} -impl_plugin!(PingPlugin, proxy, [ - ReceiveElement => Priority::Default, -]); - -impl EventHandler for PingPlugin { - fn handle(&self, evt: &ReceiveElement) -> Propagation { + fn handle_receive_element(&self, evt: &ReceiveElement) -> Propagation { let elem = &evt.0; if elem.is("iq", ns::CLIENT) && elem.attr("type") == Some("get") { if elem.has_child("ping", ns::PING) { @@ -64,3 +58,7 @@ impl EventHandler for PingPlugin { Propagation::Continue } } + +impl_plugin!(PingPlugin, proxy, [ + (ReceiveElement, Priority::Default) => handle_receive_element, +]); diff --git a/src/plugins/stanza.rs b/src/plugins/stanza.rs index 60d39f0b..02ef1259 100644 --- a/src/plugins/stanza.rs +++ b/src/plugins/stanza.rs @@ -1,7 +1,7 @@ use std::convert::TryFrom; use plugin::PluginProxy; -use event::{Event, EventHandler, ReceiveElement, Propagation, Priority}; +use event::{Event, ReceiveElement, Propagation, Priority}; use ns; pub use xmpp_parsers::message::Message; @@ -22,14 +22,8 @@ impl StanzaPlugin { proxy: PluginProxy::new(), } } -} -impl_plugin!(StanzaPlugin, proxy, [ - ReceiveElement => Priority::Default, -]); - -impl EventHandler for StanzaPlugin { - fn handle(&self, evt: &ReceiveElement) -> Propagation { + fn handle_receive_element(&self, evt: &ReceiveElement) -> Propagation { let elem = &evt.0; // TODO: make the handle take an Element instead of a reference. @@ -49,3 +43,7 @@ impl EventHandler for StanzaPlugin { Propagation::Continue } } + +impl_plugin!(StanzaPlugin, proxy, [ + (ReceiveElement, Priority::Default) => handle_receive_element, +]);