Merge remote-tracking branch 'origin/imap-locks' into exp1

python/src/deltachat/__init__.py

@@ -43,6 +43,9 @@ def py_dc_callback(ctx, evt, data1, data2):
 
     try:
         ret = callback(ctx, evt_name, data1, data2)
+        if ret is None:
+            ret = 0
+        assert isinstance(ret, int), repr(ret)
         if event_sig_types & 4:
             return ffi.cast('uintptr_t', ret)
         elif event_sig_types & 8:

python/src/deltachat/account.py

@@ -23,13 +23,14 @@ class Account(object):
     by the underlying deltachat c-library.  All public Account methods are
     meant to be memory-safe and return memory-safe objects.
     """
-    def __init__(self, db_path, logid=None):
+    def __init__(self, db_path, logid=None, eventlogging=True):
         """ initialize account object.
 
         :param db_path: a path to the account database. The database
                         will be created if it doesn't exist.
         :param logid: an optional logging prefix that should be used with
                       the default internal logging.
+        :param eventlogging: if False no eventlogging and no context callback will be configured
         """
         self._dc_context = ffi.gc(
             lib.dc_context_new(lib.py_dc_callback, ffi.NULL, ffi.NULL),
@@ -39,12 +40,16 @@ class Account(object):
             db_path = db_path.encode("utf8")
         if not lib.dc_open(self._dc_context, db_path, ffi.NULL):
             raise ValueError("Could not dc_open: {}".format(db_path))
-        self._evlogger = EventLogger(self._dc_context, logid)
-        deltachat.set_context_callback(self._dc_context, self._process_event)
+        if eventlogging:
+            self._evlogger = EventLogger(self._dc_context, logid)
+            deltachat.set_context_callback(self._dc_context, self._process_event)
         self._threads = IOThreads(self._dc_context)
         self._configkeys = self.get_config("sys.config_keys").split()
         self._imex_completed = threading.Event()
 
+    def __del__(self):
+        self.shutdown()
+
     def _check_config_key(self, name):
         if name not in self._configkeys:
             raise KeyError("{!r} not a valid config key, existing keys: {!r}".format(
@@ -333,12 +338,22 @@ class Account(object):
         lib.dc_stop_ongoing_process(self._dc_context)
         self._threads.stop(wait=wait)
 
+    def shutdown(self, wait=True):
+        """ stop threads and close and remove underlying dc_context and callbacks. """
+        if hasattr(self, "_dc_context"):
+            self.stop_threads(wait=False)  # to interrupt idle and tell python threads to stop
+            lib.dc_close(self._dc_context)
+            self.stop_threads(wait=wait)  # to wait for threads
+            deltachat.clear_context_callback(self._dc_context)
+            del self._dc_context
+
     def _process_event(self, ctx, evt_name, data1, data2):
         assert ctx == self._dc_context
-        self._evlogger(evt_name, data1, data2)
-        method = getattr(self, "on_" + evt_name.lower(), None)
-        if method is not None:
-            method(data1, data2)
+        if hasattr(self, "_evlogger"):
+            self._evlogger(evt_name, data1, data2)
+            method = getattr(self, "on_" + evt_name.lower(), None)
+            if method is not None:
+                method(data1, data2)
         return 0
 
     def on_dc_event_imex_progress(self, data1, data2):
@@ -378,12 +393,18 @@ class IOThreads:
     def imap_thread_run(self):
         while not self._thread_quitflag:
             lib.dc_perform_imap_jobs(self._dc_context)
+            if self._thread_quitflag:
+                break
             lib.dc_perform_imap_fetch(self._dc_context)
+            if self._thread_quitflag:
+                break
             lib.dc_perform_imap_idle(self._dc_context)
 
     def smtp_thread_run(self):
         while not self._thread_quitflag:
             lib.dc_perform_smtp_jobs(self._dc_context)
+            if self._thread_quitflag:
+                break
             lib.dc_perform_smtp_idle(self._dc_context)
 
 
@@ -414,7 +435,7 @@ class EventLogger:
             raise ValueError("{}({!r},{!r})".format(*ev))
         return ev
 
-    def get_matching(self, event_name_regex):
+    def get_matching(self, event_name_regex, check_error=True):
         self._log("-- waiting for event with regex: {} --".format(event_name_regex))
         rex = re.compile("(?:{}).*".format(event_name_regex))
         while 1:

python/tests/conftest.py

@@ -16,12 +16,22 @@ def pytest_addoption(parser):
     )
 
 
+@pytest.hookimpl(trylast=True)
+def pytest_runtest_call(item):
+    # perform early finalization because we otherwise get cloberred
+    # output from concurrent threads printing between execution
+    # of the test function and the teardown phase of that test function
+    if "acfactory" in item.funcargs:
+        acfactory = item.funcargs["acfactory"]
+        acfactory.finalize()
+
+
 def pytest_report_header(config, startdir):
     t = tempfile.mktemp()
     try:
-        ac = Account(t)
+        ac = Account(t, eventlogging=False)
         info = ac.get_info()
-        del ac
+        ac.shutdown()
     finally:
         os.remove(t)
     return "Deltachat core={} sqlite={}".format(
@@ -52,7 +62,6 @@ def acfactory(pytestconfig, tmpdir, request):
             self.live_count = 0
             self.offline_count = 0
             self._finalizers = []
-            request.addfinalizer(self.finalize)
             self.init_time = time.time()
 
         def finalize(self):
@@ -78,6 +87,7 @@ def acfactory(pytestconfig, tmpdir, request):
             ac = Account(tmpdb.strpath, logid="ac{}".format(self.offline_count))
             ac._evlogger.init_time = self.init_time
             ac._evlogger.set_timeout(2)
+            self._finalizers.append(ac.shutdown)
             return ac
 
         def get_configured_offline_account(self):
@@ -103,7 +113,7 @@ def acfactory(pytestconfig, tmpdir, request):
             ac._evlogger.set_timeout(30)
             ac.configure(**configdict)
             ac.start_threads()
-            self._finalizers.append(lambda: ac.stop_threads(wait=False))
+            self._finalizers.append(ac.shutdown)
             return ac
 
         def clone_online_account(self, account):
@@ -114,7 +124,7 @@ def acfactory(pytestconfig, tmpdir, request):
             ac._evlogger.set_timeout(30)
             ac.configure(addr=account.get_config("addr"), mail_pw=account.get_config("mail_pw"))
             ac.start_threads()
-            self._finalizers.append(lambda: ac.stop_threads(wait=False))
+            self._finalizers.append(ac.shutdown)
             return ac
 
     return AccountMaker()

python/tests/test_lowlevel.py

@@ -1,6 +1,8 @@
 from __future__ import print_function
 import pytest
-from deltachat import capi, Account, const
+from deltachat import capi, Account, const, set_context_callback, clear_context_callback
+from deltachat.capi import ffi
+from deltachat.account import EventLogger
 
 
 def test_empty_context():
@@ -8,6 +10,26 @@ def test_empty_context():
     capi.lib.dc_close(ctx)
 
 
+def test_callback_None2int():
+    ctx = capi.lib.dc_context_new(capi.lib.py_dc_callback, ffi.NULL, ffi.NULL)
+    set_context_callback(ctx, lambda *args: None)
+    capi.lib.dc_close(ctx)
+    clear_context_callback(ctx)
+
+
+def test_dc_close_events():
+    ctx = capi.lib.dc_context_new(capi.lib.py_dc_callback, ffi.NULL, ffi.NULL)
+    evlog = EventLogger(ctx)
+    evlog.set_timeout(5)
+    set_context_callback(ctx, lambda ctx, evt_name, data1, data2: evlog(evt_name, data1, data2))
+    capi.lib.dc_close(ctx)
+    # test that we get events from dc_close
+    print(evlog.get_matching("DC_EVENT_INFO", check_error=False))
+    print(evlog.get_matching("DC_EVENT_INFO", check_error=False))
+    print(evlog.get_matching("DC_EVENT_INFO", check_error=False))
+    print(evlog.get_matching("DC_EVENT_INFO", check_error=False))
+
+
 def test_wrong_db(tmpdir):
     tmpdir.join("hello.db").write("123")
     with pytest.raises(ValueError):

python/tox.ini

@@ -52,6 +52,7 @@ commands =
 python_files = tests/test_*.py 
 norecursedirs = .tox 
 xfail_strict=true
+timeout = 60
 
 [flake8]
 max-line-length = 120

src/context.rs

@@ -133,7 +133,7 @@ pub fn dc_context_new(
     userdata: *mut libc::c_void,
     os_name: *const libc::c_char,
 ) -> Context {
-    Context {
+    let context = Context {
         blobdir: Arc::new(RwLock::new(std::ptr::null_mut())),
         dbfile: Arc::new(RwLock::new(std::ptr::null_mut())),
         inbox: Arc::new(RwLock::new({
@@ -177,7 +177,8 @@ pub fn dc_context_new(
         ))),
         probe_imap_network: Arc::new(RwLock::new(0)),
         perform_inbox_jobs_needed: Arc::new(RwLock::new(0)),
-    }
+    };
+    context
 }
 
 unsafe fn cb_receive_imf(
@@ -297,13 +298,17 @@ pub unsafe fn dc_context_unref(context: &mut Context) {
 }
 
 pub unsafe fn dc_close(context: &Context) {
+    println!("disconnecting inbox watch yooaa");
+    info!(context, 0, "disconnecting INBOX-watch",);
     context.inbox.read().unwrap().disconnect(context);
+    info!(context, 0, "disconnecting sentbox-thread",);
     context
         .sentbox_thread
         .read()
         .unwrap()
         .imap
         .disconnect(context);
+    info!(context, 0, "disconnecting mvbox-thread",);
     context
         .mvbox_thread
         .read()
@@ -311,6 +316,7 @@ pub unsafe fn dc_close(context: &Context) {
         .imap
         .disconnect(context);
 
+    info!(context, 0, "disconnecting SMTP");
     context.smtp.clone().lock().unwrap().disconnect();
 
     context.sql.close(context);

src/dc_job.rs

@@ -897,6 +897,7 @@ pub fn dc_job_kill_action(context: &Context, action: libc::c_int) -> bool {
 
 pub unsafe fn dc_perform_imap_fetch(context: &Context) {
     let inbox = context.inbox.read().unwrap();
+    info!(context, 0, "dc_perform_imap_fetch got inbox");
     let start = clock();
 
     if 0 == connect_to_inbox(context, &inbox) {

src/imap.rs

@@ -469,6 +469,7 @@ impl Imap {
     }
 
     fn unsetup_handle(&self, context: &Context) {
+        info!(context, 0, "IMAP unsetup_handle starts");
         let session = self.session.lock().unwrap().take();
         if session.is_some() {
             match session.unwrap().close() {
@@ -478,6 +479,7 @@ impl Imap {
                 }
             }
         }
+        info!(context, 0, "IMAP unsetup_handle2.");
         let stream = self.stream.write().unwrap().take();
         if stream.is_some() {
             match stream.unwrap().shutdown(net::Shutdown::Both) {
@@ -488,6 +490,7 @@ impl Imap {
             }
         }
 
+        info!(context, 0, "IMAP unsetup_handle3.");
         let mut cfg = self.config.write().unwrap();
         cfg.selected_folder = None;
         cfg.selected_mailbox = None;
@@ -551,7 +554,6 @@ impl Imap {
                         s += c;
                         s
                     });
-
                     log_event!(
                         context,
                         Event::IMAP_CONNECTED,
@@ -565,8 +567,8 @@ impl Imap {
                     config.can_idle = can_idle;
                     config.has_xlist = has_xlist;
                     *self.connected.lock().unwrap() = true;
-
                     1
+
                 } else {
                     self.unsetup_handle(context);
                     self.free_connect_params();
@@ -594,7 +596,7 @@ impl Imap {
     }
 
     pub fn fetch(&self, context: &Context) -> libc::c_int {
-        if !self.is_connected() {
+        if !self.is_connected() || !context.sql.is_open() {
             return 0;
         }