diff --git a/src/context.rs b/src/context.rs index 13257f6..30f9d29 100644 --- a/src/context.rs +++ b/src/context.rs @@ -27,9 +27,6 @@ impl Context { #[cfg(test)] mod tests { - use std::sync::Arc; - use std::thread; - use super::Context; #[test] diff --git a/src/dispatcher.rs b/src/dispatcher.rs index 1ac85b5..5b720f6 100644 --- a/src/dispatcher.rs +++ b/src/dispatcher.rs @@ -1,8 +1,7 @@ //! This module defines the central message dispatcher to the client process. -use std::io; +use std::fmt::Debug; -use crate::context::Context; use crate::executor::Job; use crate::message_handler::MessageHandler; use crate::proto::server::ServerResponse; @@ -25,9 +24,20 @@ impl DispatchedMessage { } } -impl + Send> Job for DispatchedMessage { - fn execute(self: Box, context: &Context) -> io::Result<()> { - self.handler.run(context, self.message) +impl Job for DispatchedMessage +where + M: Debug + Send, + H: Debug + Send + MessageHandler, +{ + fn execute(self: Box) { + if let Err(error) = self.handler.run(&self.message) { + error!( + "Error in handler {}: {:?}\nMessage: {:?}", + H::name(), + error, + &self.message + ); + } } } diff --git a/src/executor.rs b/src/executor.rs index df32058..8cbf0a4 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -1,38 +1,28 @@ -use std::io; -use std::sync::Arc; +//! This module provides a facade for an abstract concurrent job executor. +//! +//! Mostly here to insulate the rest of this crate from the exact details of +//! the executor implementation. use threadpool; -use crate::context::Context; - /// Default number of threads spawned by Executor instances const NUM_THREADS: usize = 8; /// The trait of objects that can be run by an Executor. -/// -/// NOTE: Intended to be used by boxed objects, so that self's contents can be -/// moved by `execute` without running into "unknown size at compiled time" -/// E0161 errors. pub trait Job: Send { - /// Executes self in the given context. - /// Errors do not crash the process, but are error-logged. - fn execute(self: Box, context: &Context) -> io::Result<()>; + fn execute(self: Box); } -/// The central executor object that drives the client process. +/// A concurrent job execution engine. pub struct Executor { - /// The context against which jobs are executed. - context: Arc, - /// Executes the jobs. pool: threadpool::ThreadPool, } impl Executor { - /// Builds a new executor with an empty context a default number of threads. + /// Builds a new executor with a default number of threads. pub fn new() -> Self { Self { - context: Arc::new(Context::new()), pool: threadpool::Builder::new() .num_threads(NUM_THREADS) .thread_name("Executor".to_string()) @@ -42,36 +32,25 @@ impl Executor { /// Schedules execution of the given job on this executor. pub fn schedule(&self, job: Box) { - let context = self.context.clone(); - self.pool.execute(move || { - if let Err(error) = job.execute(&*context) { - error!("Executable returned error: {:?}", error) - } - }) + self.pool.execute(move || job.execute()); } - /// Blocks until all scheduled jobs are executed, then returns the context. - pub fn join(self) -> Context { + /// Blocks until all scheduled jobs are executed. + pub fn join(self) { self.pool.join(); - - // Once the pool is joined, no-one should be holding on to copies of - // `self.context` anymore, so we unwrap() here. - Arc::try_unwrap(self.context).unwrap() } } #[cfg(test)] mod tests { use std::io; - use std::sync::{Arc, Barrier, Mutex}; + use std::sync::{Arc, Barrier}; - use super::{Context, Executor, Job}; + use super::{Executor, Job}; #[test] - fn immediate_join_returns_empty_context() { - let context = Executor::new().join(); - assert_eq!(context.users.lock().get_list(), vec![]); - assert_eq!(context.rooms.lock().get_room_list(), vec![]); + fn immediate_join() { + Executor::new().join() } struct Waiter { @@ -79,9 +58,8 @@ mod tests { } impl Job for Waiter { - fn execute(self: Box, context: &Context) -> io::Result<()> { + fn execute(self: Box) { self.barrier.wait(); - Ok(()) } } @@ -91,20 +69,13 @@ mod tests { let barrier = Arc::new(Barrier::new(2)); - let waiter1 = Box::new(Waiter { + executor.schedule(Box::new(Waiter { barrier: barrier.clone(), - }); - let waiter2 = Box::new(Waiter { + })); + executor.schedule(Box::new(Waiter { barrier: barrier.clone(), - }); + })); - executor.schedule(waiter1); - executor.schedule(waiter2); - - let context = executor.join(); - assert_eq!(context.users.lock().get_list(), vec![]); - assert_eq!(context.rooms.lock().get_room_list(), vec![]); + executor.join(); } - - // TODO: Add a test that exercises modifying Context. } diff --git a/src/message_handler.rs b/src/message_handler.rs index 7a4a4c8..bec2d57 100644 --- a/src/message_handler.rs +++ b/src/message_handler.rs @@ -1,11 +1,14 @@ +use std::fmt::Debug; use std::io; -use crate::context::Context; - /// A trait for types that can handle reception of a message. /// /// Message types are mapped to handler types by Dispatcher. /// This trait is intended to allow composing handler logic. -pub trait MessageHandler { - fn run(self, context: &Context, message: Message) -> io::Result<()>; +pub trait MessageHandler: Debug { + /// Attempts to handle the given message. + fn run(self, message: &Message) -> io::Result<()>; + + /// Returns the name of this handler type. + fn name() -> String; }