Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crash in core, sqlite3_step(), probably related to sqlx #1129

Closed
r10s opened this issue Apr 7, 2021 · 13 comments
Closed

crash in core, sqlite3_step(), probably related to sqlx #1129

r10s opened this issue Apr 7, 2021 · 13 comments
Labels

Comments

@r10s
Copy link
Member

r10s commented Apr 7, 2021

i encountered some crashes the last days, all similar to https://gist.github.com/r10s/f88d4305a8cbaf91861b3b0298e60a5f (i have more logs, if needed)

Details:
Name: SIGSEGV signal
Date: 7. Apr 2021 at 03:26:53
Stack trace:
0 DBDebugToolkit 0x0000000104ca71d0 handleSIGSEGVSignal + 88
1 libsystem_platform.dylib 0x00000001f8a25d90 3A71914A-C2A7-3514-B519-DF319E7A6E02 + 28048
2 DcCore 0x0000000105a7e374 sqlite3VdbeExec + 32576
3 DcCore 0x0000000105a42d20 sqlite3_step + 2600
4 DcCore 0x00000001059af230 _ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17h0d3631d5f7ad68c2E + 4716
5 DcCore 0x00000001059aa160 _ZN4core3ops8function6FnOnce40call_once$u7b$$u7b$vtable.shim$u7d$$u7d$17h8c5b78c51826ee8aE + 168
6 DcCore 0x00000001059e65d8 _ZN3std3sys4unix6thread6Thread3new12thread_start17hc2fb1317c6a3263bE + 36
7 libsystem_pthread.dylib 0x00000001f8a33c9c _pthread_start + 288
8 libsystem_pthread.dylib 0x00000001f8a38880 thread_start + 8

there is a pr on sqlx that is also related to fetching, launchbadge/sqlx#1156, might be that issue.

@r10s r10s added the bug label Apr 7, 2021
@link2xt
Copy link
Contributor

link2xt commented Apr 7, 2021

This looks like SQLite statement is used after finalizing it.

New PR is very likely to work around this issue, because it re-enables statement cache, which means the statements will be finalized much later than their use stops.

@r10s
Copy link
Member Author

r10s commented Apr 7, 2021

hm, it crashed again. https://gist.github.com/r10s/6f9203f52258ad72667acbf41863a075

i will double-check, that i was really using the updated code. also, having a closer look, it seems to be related to performFetchWithCompletionHandler in all cases, maybe we can be more careful on ios as well here.

@link2xt
Copy link
Contributor

link2xt commented Apr 7, 2021

Could be related: launchbadge/sqlx#634

@r10s
Copy link
Member Author

r10s commented Apr 7, 2021

got a crash again, https://gist.github.com/r10s/1f8a9e93b8f8e9acf477fb3d5611e68f - interesting bit, also for the crash reported above is that the two "wakeup" entry points are called roughly at the same time:

17:06:54.519 💙 INFO ➡️ performFetchWithCompletionHandler
17:06:54.521 💙 INFO ➡️ Notifications: didReceiveRemoteNotification

this was also true on rusqlite, however, maybe this causes problems now as nearly the same code is executed at the same time (startIo, followed by maybeNetwork, then the fetching). otoh, log looks good at a first glance, the second startIo call is ignored.

unfortunately, the log is not that helpful as it misses the minutes before the crash :/ bugs everywhere, here in the debug toolkit ... or i missed sth.

@link2xt
Copy link
Contributor

link2xt commented Apr 7, 2021

Probably fixed by deltachat/deltachat-core-rust#2336 as the statements will be kept around in the cache for a while and unlikely to be finalized before the last sqlite3_step is run on them.

@r10s
Copy link
Member Author

r10s commented Apr 8, 2021

the fix runs on ios for more than 13h uptime now, i think, we can consider this issue as being fixed :)

thanks again for fixing, @link2xt

@r10s r10s closed this as completed Apr 8, 2021
@r10s
Copy link
Member Author

r10s commented Apr 8, 2021

reopening as i got a crash again, much less often as before, indeed. and ios is also kind of stressing core due to #1085, maybe crashes disappear completely when #1085 is fixed. but at the end, there is still an open issue with sqlx.

recent stack: https://gist.github.com/r10s/d868357fa320b814ef89ef688196b70e (log incomplete, see #1131)

@r10s
Copy link
Member Author

r10s commented Apr 11, 2021

did not get any more crashes with the force-persistent queries, closing and fingers crossed ...

@r10s r10s closed this as completed Apr 11, 2021
@r10s
Copy link
Member Author

r10s commented Apr 17, 2021

unfortunately, i got this crash again, it is less often, but it is still there.

also, what is not-so-nice, if that happens during a background-fetch, we are not calling the completion handler, which may have a bad impact to how often we're getting called wrt remote-wakeup and local-wakeup. however, i just did a testflight, let's see how often crashes are reported "in the wild"

last non-sqlx version (1.17.1) only had 1 crash per week in total on some hundred installations (after we fixed several other things from 1.16 which had far more crashes):
Screen Shot 2021-04-17 at 15 24 10

@r10s r10s reopened this Apr 17, 2021
@r10s
Copy link
Member Author

r10s commented Apr 23, 2021

i still get these crashes regularily, since 1.53 (shared cache enabled) not in the background but when the app is tapped by the user and comes from background to foreground - things get stale and the app disappears after 3 seconds.

unfortunately, there is no log written, eg. DBDebugToolkit stays empty - so it is questionable, if these crashes really reach the testflight stats.

i double-checked by switching back to core51 using rusqlite - and i did not get any crash since then (in a timeframe where i got usually at least 2)

also, i can say that on my iphone7, the whole app feels more reposible, somehow, things get less fluid and more blocking between core52 and core53 (that enabled the shared cache)

@cyBerta
Copy link
Contributor

cyBerta commented Apr 26, 2021

since we switched back to rusqlite, we can close this issue for now

@cyBerta cyBerta closed this as completed Apr 26, 2021
@link2xt
Copy link
Contributor

link2xt commented Apr 26, 2021

It may also be fixed by launchbadge/sqlx#1186

@r10s
Copy link
Member Author

r10s commented Apr 26, 2021

It may also be fixed by launchbadge/sqlx#1186

i got the crash-mutant reported at #1129 (comment) once with launchbadge/sqlx#1186 in - however, as there is no log written, it is hard to tell, how and if that is related to sqlx directly - but it did never happen with rusqlite.

EDIT, a week later: still not seeing the crashes in rusqlite, so that really seems to be related to sqlx and was not fixed, only mitigated somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants