api: cffi api to create account manager with existing events channel to see events emitted during startup. dc_event_channel_new, dc_event_channel_unref, dc_event_channel_get_event_emitter and dc_accounts_new_with_event_channel

[Simon-Laux]

closes #7606

• fix broken ci tests
• test it manually

[Simon-Laux]

Is this still the case?, rust code looks like it allows multiple listeners: it uses and async broadcast.

[Hocuri]

Looks like it, probably it was forgotten in #5478 to update the documentation. But of course, the only way to be totally sure is actually trying it out.

[Simon-Laux]

Tried it, the assumption was correct: the comment is outdated, in cffi and rust api it is possible to have multiple event channels, only in jsonrpc api it is currently not possible.

[link2xt]

Cell is not thread-safe. If C side uses this from multiple threads, this can result in memory corruption when trying to write into the same Option multiple times. Better use Arc<Mutex<Option<Events>>> so it is not possible to create two account managers by initializing them at the same time.

[iequidoo]

I.e. Mutex instead of Cell and Arc instead of Box below. OTOH do we need thread safety here? We can just document that it's not thread-safe. By using Mutex we only add thread safety to the buggy scenario which shouldn't happen anyway.

[link2xt]

OTOH do we need thread safety here? We can just document that it's not thread-safe.

All CFFI API is declared as thread-safe so we don't have to think about which is safe and which is not:

core/deltachat-ffi/deltachat.h

Lines 143 to 147 in fd90493

	* ## Thread safety
	*
	* All deltachat-core functions, unless stated otherwise, are thread-safe.
	* In particular, it is safe to pass the same dc_context_t pointer
	* to multiple functions running concurrently in different threads.

It's probably not really true that all code is thread-safe because we don't have Mutexes around Message object, but that's a bug in that code, let's not add more bugs.

[link2xt]

We write this as drop(Box::from_raw(event_channel)) everywhere else.

[Simon-Laux]

yeah that's clearer, though "everywhere" is not quite correct, at-least not from the place I copied this from. could be a follow up pr to make sure it is the same everywhere, though I'm not sure how important it is for the existing calls.

[iequidoo]

Suggested change

	* which allows you see events emitted during startup.
	* which allows you to see events emitted during startup.

[iequidoo]

Suggested change

	* The account manager takes an directory
	* The account manager takes a directory

Maybe it's better not to duplicate the documentation, but just add a reference to dc_account_new() and explain the difference. Otherwise API users need to read both and build a diff in their head, and we have more text to maintain.

[Simon-Laux]

@r10s what's your opinion on references vs duplication in cffi docs?

[iequidoo]

I think it depends on amount of text. Huge texts should never be duplicated because users then have more work to read the docs and understand the difference between functions. E.g. see https://pubs.opengroup.org/onlinepubs/9799919799/functions/open.html, it documents two functions at the same time and explains which and when should be used, would be good if we can do the same, w/o duplication and extra cross-references.

[iequidoo]

I.e. Mutex instead of Cell and Arc instead of Box below. OTOH do we need thread safety here? We can just document that it's not thread-safe. By using Mutex we only add thread safety to the buggy scenario which shouldn't happen anyway.