fix up the event system, no more unsafe!

lumi created

Change summary

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(-)

Detailed changes

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)

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<E: Event>: 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<Any>,
+}
+
+impl AbstractEvent {
+    /// Creates an abstract event from a concrete event.
+    pub fn new<E: Event>(event: E) -> AbstractEvent {
+        AbstractEvent {
+            inner: Box::new(event),
+        }
+    }
+
+    /// Downcasts this abstract event into a concrete event.
+    pub fn downcast<E: Event + 'static>(&self) -> Option<&E> {
+        self.inner.downcast_ref::<E>()
+    }
+
+    /// Checks whether this abstract event is a specific concrete event.
+    pub fn is<E: Event + 'static>(&self) -> bool {
+        self.inner.is::<E>()
+    }
 }
 
 struct Record<P, T>(P, T);
@@ -49,21 +70,10 @@ pub enum Propagation {
     Continue,
 }
 
-#[derive(Debug)]
-struct GarbageEvent;
-
-impl Event for GarbageEvent {}
-
-impl<E, F> EventHandler<E> for Box<F> 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<TypeId, Vec<Record<Priority, Box<Any>>>>,
-    queue: Vec<(TypeId, Box<Any>)>,
+    handlers: BTreeMap<TypeId, Vec<Record<Priority, Box<EventHandler>>>>,
+    queue: Vec<(TypeId, AbstractEvent)>,
 }
 
 impl Dispatcher {
@@ -76,17 +86,39 @@ impl Dispatcher {
     }
 
     /// Register an event handler.
-    pub fn register<E, H>(&mut self, priority: Priority, handler: H) where E: Event + 'static, H: EventHandler<E> {
-        let handler: Box<EventHandler<E>> = Box::new(handler) as Box<EventHandler<E>>;
+    pub fn register<E, F>(&mut self, priority: Priority, func: F)
+        where
+            E: Event,
+            F: Fn(&E) -> Propagation + 'static {
+        struct Handler<E, F> where E: Event, F: Fn(&E) -> Propagation {
+            func: F,
+            _marker: PhantomData<E>,
+        }
+
+        impl<E: Event, F: Fn(&E) -> Propagation + 'static> EventHandler for Handler<E, F> {
+            fn handle(&self, evt: &AbstractEvent) -> Propagation {
+                if let Some(e) = evt.downcast::<E>() {
+                    (self.func)(e)
+                }
+                else {
+                    Propagation::Continue
+                }
+            }
+        }
+
+        let handler: Box<EventHandler> = Box::new(Handler {
+            func: func,
+            _marker: PhantomData,
+        }) as Box<EventHandler>;
         let ent = self.handlers.entry(TypeId::of::<E>())
                                .or_insert_with(|| Vec::new());
-        ent.push(Record(priority, Box::new(handler) as Box<Any>));
+        ent.push(Record(priority, handler));
         ent.sort();
     }
 
     /// Append an event to the queue.
     pub fn dispatch<E>(&mut self, event: E) where E: Event {
-        self.queue.push((TypeId::of::<E>(), Box::new(event) as Box<Any>));
+        self.queue.push((TypeId::of::<E>(), 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<GarbageEvent> = 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<E>(&mut self, event: E) where E: Event {
-        if let Some(handlers) = self.handlers.get_mut(&TypeId::of::<E>()) {
-            for &mut Record(_, ref mut handler) in handlers {
-                let h = handler.downcast_mut::<Box<EventHandler<E>>>().unwrap();
-                match h.handle(&event) {
-                    Propagation::Stop => { return; },
-                    Propagation::Continue => (),
-                }
-            }
-        }
-    }
-}
-
-pub struct EventProxy<T: ?Sized, E: Event> {
-    inner: Arc<Box<T>>,
-    vtable: *mut (),
-    _event_type: PhantomData<E>,
-}
-
-impl<T: ?Sized, E: Event> EventProxy<T, E> {
-    /// 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<E>, if it does not, refer to above
-    /// statement.
-    pub unsafe fn new<H: EventHandler<E>>(inner: Arc<Box<T>>) -> EventProxy<T, E> {
-        let box_with_vtable = &*ptr::null::<H>() as &EventHandler<E>;
-        let obj: TraitObject = mem::transmute(box_with_vtable);
-        EventProxy {
-            inner: inner,
-            vtable: obj.vtable,
-            _event_type: PhantomData,
-        }
-    }
-}
-
-impl<T: ?Sized, E: Event> EventHandler<E> for EventProxy<T, E> where Box<T>: '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<E> = unsafe { mem::transmute(obj) };
-        let prop = handler.handle(evt);
-        unsafe { Arc::<Box<T>>::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<MyEvent> 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<MyEvent> for EvilHandler {
-            fn handle(&self, _: &MyEvent) -> Propagation {
-                panic!("should not be called");
+            else {
+                Propagation::Stop
             }
-        }
+        });
 
-        impl EventHandler<MyEvent> 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,

src/lib.rs 🔗

@@ -1,4 +1,3 @@
-#![feature(raw)]
 #![feature(try_from)]
 
 extern crate xml;

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<E, H>(&self, priority: Priority, handler: H) where E: Event, H: EventHandler<E> {
+    pub fn register_handler<E, F>(&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);
         });
     }
 

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<Box<$crate::plugin::Plugin>>) {
                 $(
-                    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),*]);
     };
 }

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<ReceiveElement> 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<ReceiveElement> for MessagingPlugin {
         Propagation::Continue
     }
 }
+
+impl_plugin!(MessagingPlugin, proxy, [
+    (ReceiveElement, Priority::Default) => handle_receive_element,
+]);

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<ReceiveElement> 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<ReceiveElement> for PingPlugin {
         Propagation::Continue
     }
 }
+
+impl_plugin!(PingPlugin, proxy, [
+    (ReceiveElement, Priority::Default) => handle_receive_element,
+]);

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<ReceiveElement> 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<ReceiveElement> for StanzaPlugin {
         Propagation::Continue
     }
 }
+
+impl_plugin!(StanzaPlugin, proxy, [
+    (ReceiveElement, Priority::Default) => handle_receive_element,
+]);