deltachat-core-rust
deltachat-core-rust copied to clipboard
Unify event emitter types for context and account manager
This is a proposal on top of #3421, breaking C API to unify event emitter types for contexts and account manager.
#3421 changes only the Rust implementation, but keeps C API intact. I have an idea to break C API and make construction of event channel explicit, so library user will construct dc_event_emitter_t
even before constructing the Context
.
First advantage of this approach is that event channel construction never fails with an Error
, so it's guaranteed that even if Context
cannot be constructed, the errors can be logged via channel instead of using eprintln!
. Logging to stderr via eprintln!
is problematic in TUI applications, see discussion at #3419.
This is also useful for managing multiple contexts, especially in tests, and can be used both in test_utils.rs
and likely python tests to aggregate all logs into a single sink. Instead of aggregating events by creating multiple threads and polling from multiple event emitters, it is possible to construct a single event channel and use a single event loop for multiple contexts.
And finally, it will fix #2280 by discouraging memory-unsafe way to stop event thread by deallocating dc_context_t
and causing dc_get_next_event()
to return NULL
, as previously used by Python and I suspect still used by Node.js bindings, causing test segfaults. If a clone of the Sender
is put into dc_event_emitter_t
, user will not be able to close the sender side of the channel by destroying all contexts and dc_get_next_event()
will never return NULL
.
I haven't added compatibility macros/wrappers, the only code that needs to be changed is Android and iOS clients. Node module is already changed in this PR.
what if you kept dc_accounts_event_emitter_t
and its functions but they are just aliases for dc_event_emitter_t
. could we achieve the same without breaking the ffi?
some #define
should do the job:
#define dc_accounts_event_emitter_t dc_event_emitter_t
#define dc_accounts_get_next_event dc_get_next_event
#define dc_accounts_event_emitter_unref dc_event_emitter_unref
we could just add these lines to deltachat.h
with a comment that we will remove that in the ~next version.
this will give implementations some more time and remove the pressure, esp. when it comes to switching forth and back for whatever reason.
but - if desktop is already done i personally would also be fine without the defines, android/ios is usually easier to adapt anyway and i know what to do there :)
but there is also python , that probably needs adaotion as well? so, if these three defines make life easier ...
I added defines for backwards compatibility. Python does not depend on them, but they will allow to build Android/iOS without changes.