refactor: make run take the callback and avoid boxing

examples/simple.rs

@@ -30,15 +30,14 @@ fn main() {
     let dir = tempdir().unwrap();
     let dbfile = dir.path().join("db.sqlite");
     println!("creating database {:?}", dbfile);
-    let ctx =
+    let ctx = Context::new("FakeOs".into(), dbfile).expect("Failed to create context");
-        Context::new(Box::new(cb), "FakeOs".into(), dbfile).expect("Failed to create context");
     let info = ctx.get_info();
     let duration = time::Duration::from_millis(8000);
     println!("info: {:#?}", info);
 
     crossbeam::scope(|s| {
         let t1 = s.spawn(|_| {
-            ctx.run();
+            ctx.run(cb);
         });
 
         println!("configuring");

src/context.rs

@@ -37,7 +37,7 @@ use crate::sql::Sql;
 /// * `data2` - Depends on the event parameter, see [Event].
 pub type ContextCallback = dyn Fn(&Context, Event) -> () + Send + Sync;
 
-#[derive(DebugStub)]
+#[derive(Debug)]
 pub struct Context {
     /// Database file path
     dbfile: PathBuf,
@@ -61,9 +61,6 @@ pub struct Context {
     pub generating_key_mutex: Mutex<()>,
     pub translated_stockstrings: RwLock<HashMap<usize, String>>,
 
-    #[debug_stub = "Callback"]
-    cb: Box<ContextCallback>,
-
     event_sender: Sender<Event>,
     event_receiver: Receiver<Event>,
     shutdown_sender: Sender<()>,
@@ -105,9 +102,7 @@ macro_rules! while_running {
 
 impl Context {
     /// Creates new context.
-    pub fn new(cb: Box<ContextCallback>, os_name: String, dbfile: PathBuf) -> Result<Context> {
+    pub fn new(os_name: String, dbfile: PathBuf) -> Result<Context> {
-        pretty_env_logger::try_init_timed().ok();
-
         let mut blob_fname = OsString::new();
         blob_fname.push(dbfile.file_name().unwrap_or_default());
         blob_fname.push("-blobs");
@@ -115,15 +110,10 @@ impl Context {
         if !blobdir.exists() {
             std::fs::create_dir_all(&blobdir)?;
         }
-        Context::with_blobdir(cb, os_name, dbfile, blobdir)
+        Context::with_blobdir(os_name, dbfile, blobdir)
     }
 
-    pub fn with_blobdir(
+    pub fn with_blobdir(os_name: String, dbfile: PathBuf, blobdir: PathBuf) -> Result<Context> {
-        cb: Box<ContextCallback>,
-        os_name: String,
-        dbfile: PathBuf,
-        blobdir: PathBuf,
-    ) -> Result<Context> {
         ensure!(
             blobdir.is_dir(),
             "Blobdir does not exist: {}",
@@ -136,7 +126,6 @@ impl Context {
         let ctx = Context {
             blobdir,
             dbfile,
-            cb,
             os_name: Some(os_name),
             running_state: Arc::new(RwLock::new(Default::default())),
             sql: Sql::new(),
@@ -195,9 +184,13 @@ impl Context {
     }
 
     /// Start the run loop.
-    pub fn run(&self) {
+    pub fn run<F>(&self, cb: F)
-        use crossbeam::channel::select;
+    where
+        F: Fn(&Context, Event) -> () + Send + Sync,
+    {
+        // TODO: ensure this can be only called once.
 
+        use crossbeam::channel::select;
         self.is_running.store(true, Ordering::Relaxed);
 
         crossbeam::scope(|s| {
@@ -240,7 +233,7 @@ impl Context {
                     recv(self.event_receiver) -> event => {
                         // This gurantees that the callback is always called from the thread
                         // that called `run`.
-                        (*self.cb)(self, event.unwrap())
+                        cb(self, event.unwrap())
                     },
                     recv(self.shutdown_receiver) -> _ => break,
                 }
@@ -639,7 +632,7 @@ mod tests {
         let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
         std::fs::write(&dbfile, b"123").unwrap();
-        let res = Context::new(Box::new(|_, _| ()), "FakeOs".into(), dbfile);
+        let res = Context::new("FakeOs".into(), dbfile);
         assert!(res.is_err());
     }
 
@@ -654,7 +647,7 @@ mod tests {
     fn test_blobdir_exists() {
         let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
-        Context::new(Box::new(|_, _| ()), "FakeOS".into(), dbfile).unwrap();
+        Context::new("FakeOS".into(), dbfile).unwrap();
         let blobdir = tmp.path().join("db.sqlite-blobs");
         assert!(blobdir.is_dir());
     }
@@ -665,7 +658,7 @@ mod tests {
         let dbfile = tmp.path().join("db.sqlite");
         let blobdir = tmp.path().join("db.sqlite-blobs");
         std::fs::write(&blobdir, b"123").unwrap();
-        let res = Context::new(Box::new(|_, _| ()), "FakeOS".into(), dbfile);
+        let res = Context::new("FakeOS".into(), dbfile);
         assert!(res.is_err());
     }
 
@@ -675,7 +668,7 @@ mod tests {
         let subdir = tmp.path().join("subdir");
         let dbfile = subdir.join("db.sqlite");
         let dbfile2 = dbfile.clone();
-        Context::new(Box::new(|_, _| ()), "FakeOS".into(), dbfile).unwrap();
+        Context::new("FakeOS".into(), dbfile).unwrap();
         assert!(subdir.is_dir());
         assert!(dbfile2.is_file());
     }
@@ -685,7 +678,7 @@ mod tests {
         let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
         let blobdir = PathBuf::new();
-        let res = Context::with_blobdir(Box::new(|_, _| ()), "FakeOS".into(), dbfile, blobdir);
+        let res = Context::with_blobdir("FakeOS".into(), dbfile, blobdir);
         assert!(res.is_err());
     }
 
@@ -694,7 +687,7 @@ mod tests {
         let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
         let blobdir = tmp.path().join("blobs");
-        let res = Context::with_blobdir(Box::new(|_, _| ()), "FakeOS".into(), dbfile, blobdir);
+        let res = Context::with_blobdir("FakeOS".into(), dbfile, blobdir);
         assert!(res.is_err());
     }
 

src/test_utils.rs

@@ -2,6 +2,8 @@
 //!
 //! This module is only compiled for test runs.
 
+use std::sync::Arc;
+
 use tempfile::{tempdir, TempDir};
 
 use crate::config::Config;
@@ -15,7 +17,7 @@ use crate::key;
 /// The temporary directory can be used to store the SQLite database,
 /// see e.g. [test_context] which does this.
 pub struct TestContext {
-    pub ctx: Context,
+    pub ctx: Arc<Context>,
     pub dir: TempDir,
 }
 
@@ -32,7 +34,13 @@ pub fn test_context(callback: Option<Box<ContextCallback>>) -> TestContext {
         Some(cb) => cb,
         None => Box::new(|_, _| ()),
     };
-    let ctx = Context::new(cb, "FakeOs".into(), dbfile).unwrap();
+    let ctx = Arc::new(Context::new("FakeOs".into(), dbfile).unwrap());
+    let ctx_1 = ctx.clone();
+
+    std::thread::spawn(move || {
+        ctx_1.run(|context, event| (*cb)(context, event));
+    });
+
     TestContext { ctx, dir }
 }
 

tests/stress.rs

@@ -1,6 +1,7 @@
 //! Stress some functions for testing; if used as a lib, this file is obsolete.
 
 use std::collections::HashSet;
+use std::sync::Arc;
 
 use deltachat::config;
 use deltachat::context::*;
@@ -207,14 +208,20 @@ fn cb(_context: &Context, _event: Event) {}
 
 #[allow(dead_code)]
 struct TestContext {
-    ctx: Context,
+    ctx: Arc<Context>,
     dir: TempDir,
 }
 
 fn create_test_context() -> TestContext {
     let dir = tempdir().unwrap();
     let dbfile = dir.path().join("db.sqlite");
-    let ctx = Context::new(Box::new(cb), "FakeOs".into(), dbfile).unwrap();
+    let ctx = Arc::new(Context::new("FakeOs".into(), dbfile).unwrap());
+    let ctx_1 = ctx.clone();
+
+    std::thread::spawn(move || {
+        ctx_1.run(cb);
+    });
+
     TestContext { ctx, dir }
 }