From cd4d265055e8e0d55c4227a1f40465da5cbedbe3 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Wed, 4 May 2022 11:27:57 +0200 Subject: [PATCH] safer handling of calling account hooks, refined shutdown comment --- python/src/deltachat/account.py | 3 ++- python/src/deltachat/events.py | 32 +++++++++++++++++++------------- python/tests/test_lowlevel.py | 12 ++++++++++++ 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/python/src/deltachat/account.py b/python/src/deltachat/account.py index 4d2d13338..77c7cb8fc 100644 --- a/python/src/deltachat/account.py +++ b/python/src/deltachat/account.py @@ -698,7 +698,8 @@ class Account(object): # mark the event thread for shutdown (latest on next incoming event) self._event_thread.mark_shutdown() - # stop_io also causes an info event that will terminate the event thread + # stop_io also causes an info event which will wake up + # the EventThread's inner loop and let it notice the shutdown marker. self.stop_io() self.log("wait for event thread to finish") diff --git a/python/src/deltachat/events.py b/python/src/deltachat/events.py index 6a3198f0f..d7520baf2 100644 --- a/python/src/deltachat/events.py +++ b/python/src/deltachat/events.py @@ -1,5 +1,8 @@ import threading +import sys +import traceback import time +import io import re import os from queue import Queue, Empty @@ -221,17 +224,13 @@ class EventThread(threading.Thread): self._inner_run() def _inner_run(self): - if self._marked_for_shutdown or self.account._dc_context is None: - return event_emitter = ffi.gc( lib.dc_get_event_emitter(self.account._dc_context), lib.dc_event_emitter_unref, ) while not self._marked_for_shutdown: event = lib.dc_get_next_event(event_emitter) - if event == ffi.NULL: - break - if self._marked_for_shutdown: + if event == ffi.NULL or self._marked_for_shutdown: break evt = lib.dc_event_get_id(event) data1 = lib.dc_event_get_data1_int(event) @@ -245,15 +244,22 @@ class EventThread(threading.Thread): lib.dc_event_unref(event) ffi_event = FFIEvent(name=evt_name, data1=data1, data2=data2) - try: - self.account._pm.hook.ac_process_ffi_event(account=self, ffi_event=ffi_event) - for name, kwargs in self._map_ffi_event(ffi_event): - self.account.log("calling hook name={} kwargs={}".format(name, kwargs)) - hook = getattr(self.account._pm.hook, name) + self.account._pm.hook.ac_process_ffi_event(account=self, ffi_event=ffi_event) + for name, kwargs in self._map_ffi_event(ffi_event): + hook = getattr(self.account._pm.hook, name) + info = "call {} kwargs={} failed".format(name, kwargs) + with self.swallow_and_log_exception(info): hook(**kwargs) - except Exception: - if not self._marked_for_shutdown and self.account._dc_context is not None: - raise + + @contextmanager + def swallow_and_log_exception(self, info): + try: + yield + except Exception as ex: + logfile = io.StringIO() + traceback.print_exception(*sys.exc_info(), file=logfile) + self.account.log("{}\nException {}\nTraceback:\n{}" + .format(info, ex, logfile.getvalue())) def _map_ffi_event(self, ffi_event: FFIEvent): name = ffi_event.name diff --git a/python/tests/test_lowlevel.py b/python/tests/test_lowlevel.py index 7be054d01..b14943f83 100644 --- a/python/tests/test_lowlevel.py +++ b/python/tests/test_lowlevel.py @@ -140,3 +140,15 @@ def test_get_info_open(tmpdir): info = cutil.from_dc_charpointer(lib.dc_get_info(ctx)) assert 'deltachat_core_version' in info assert 'database_dir' in info + + +def test_logged_hook_failure(acfactory): + ac1 = acfactory.get_pseudo_configured_account() + cap = [] + ac1.log = cap.append + with ac1._event_thread.swallow_and_log_exception("some"): + 0/0 + assert cap + assert "some" in str(cap) + assert "ZeroDivisionError" in str(cap) + assert "Traceback" in str(cap)