try to fix notifications

[Simon-Laux]

• don't use logger from context outside of context -> r10s did that in add global logger to DcCore and DcShare #1994
• context wrapper objects that are created for each event can not be used to save state so I commented out those non working statements -> now also in remove unused, not working debug variables from context #1976
• try to add real shutdown for event handler and closing the db (Notifications: Crash in background activity -> File Locks #1979)

Does not crash as often anymore, but:

• delay after waking up / unsuspeding, link2xt said probably because of missing cache, also might just be sqlcipher taking long to open the db
• events are not received by the UI anymore, althrough I sucessfully restarted the handler that the events are printed to the console.
• there is a performance warning because I wait in the main thread for the event thread to finish: "Thread Performance Checker: Thread running at QOS_CLASS_USER_INTERACTIVE waiting on a lower QoS thread running at QOS_CLASS_BACKGROUND. Investigate ways to avoid priority inversions"
• TODO: eventHandlerActive boolean should proabably be an atomic? -> seems to be not necessary
• it wastes time when going in the background, sometimes just waits while doing nothing, Dedicated Background Fetch function deltachat-core-rust#4420 could be useful here. - though keeping open a bit for the case that user comes back to the app is a feature, not a bug

Also See

• Dedicated Background Fetch function deltachat-core-rust#4420

closes #1979
closes #1830
closes #1202 (basically the same as #1979)

if crashes still continue, we should open new, on-point issues

[r10s]

great to push this forward and to have a fresh look at the current state. thanks a lot!

i know, this is a draft, but still:

for easier review, merging and more on point discussions, please do not put unrelated and many things into one pull requests. small pull requests, fixing a precisely formulated issues are easier to review are much more probable to get in soon (eg. the logging could be one pr)

also, please do not shift blocks around, this also makes it very hard to find the gist of the change. this whole area is very sensible and complicated already, and one does not always is in the correct mindset, so keep it simple

[Simon-Laux]

yeah would have made more sense to split the changes over multiple commits.
creating a dedicated pr for moving the logger away from context is a good idea, by the way it it still needed that it is pluggable/changable? or could we just import the simple logger everywhere?

Edit: made an issue for the logging discussion: #1973

Edit2: made two prs about reducing context dependence in the event loop: #1978, #1976 (after those are merged I can rebase this pr to make it simpler, but the biggest impact in simplification would be a pr that deals with #1973)

[Simon-Laux]

seems like this fixed the resume issue

it did not, just got it again (now event handler is false though)

[Simon-Laux]

since last commit resume works reliable for me.

[r10s]

i simplified the logging part at #1994 - once #1994 is merged, this pr should be rebased and become much simpler

[r10s]

this PR in its current form leads to issues when the app is restarted after being suspended to memory before (so if applicationWillTerminate() is not called and we land in maybeStart() on restart):

in this case, the whole app stays in memory - while all context objects get invalidated by closeDB() EDIT: after chat with @link2xt, they're not invalidated, but still there are other contexts objects after result.

later in maybeStart(), the accounts object is recreated, with new context objects. leading to weird behaviours or crashes as parts of the app use the old context objects while other parts use the new

the account suspend/resume idea from before would have avoided that situation.

otoh, suspend/resume would introduce another state in core, which will have issues on its own, so i tend to follow the approach starting in this PR of calling dc_accounts_unref() on suspend.

so, this PR is necessary, but not sufficient: additionally, we need to invalidate all context objects returned by dc_accounts_get*() - maybe that is not that-hard as the swift wrapper objects can stay as is, we could eg. use a function to get the account object that fulfils the conditions

[r10s]

after some chatting with core devs deep into things, this PR does basically two things:

1. graceful shutdown event handler, fix crashes with previous shutdown of event handlers

2. avoid 0xdead10cc crashes that happens as the account's lock file is kept open to avoid only one account instance to add accounts. these 0xdead10cc crashes are avoided by calling dc_accounts_unref() - which has side effects and other issues as described above

other than i have thought initially, this PR does not change anything on dc_context_t and possible 0xdead10cc crashes related to them: accounts that you get with dc_accounts_get_*() should stay valid even after dc_accounts_unref() they are not closed or so; you can still use them and have to dc_context_unref() them in the end

todo:

• dc_accounts_t 0xdead10cc crashes should be fixed by use new core that avoids 0xdead10cc on dc_accounts_t #2009

• dc_context_t 0xdead10cc crashes are at least partly related to encrypted accounts; this will be fixed by disable creation of new encrypted accounts, fade out existing ones #2010

• this PR should be closed (kept for reference!) and being replaced by a PR that cherry picks "1. only graceful shutdown event" handler that closes stop event loop in a thread safe way #1830 then