sqlite: fix a couple segfaults

[abonander]

Combines changes from #1314 and #1186 and adds the repro from #1300.

• StatementHandle is wrapped in an Arc so that it doesn't call sqlite3_finalize() while the worker is still calling sqlite3_step().
• sqlite3_reset() is now only called within the context of the StatementWorker so that it cannot occur concurrently with a call to sqlite3_step() (instead of using a mutex to serialize these calls like Fix segfault in sqlite when a future is aborted #1314)

CC @madadam @link2xt can you please try this branch to see if it fixes the segfaults ya'll were seeing?

closes #1300
closes #1186
closes #1314

[madadam]

Does this need to take mut (also in step) ?

[abonander]

Strictly speaking: no. However, these are only ever called from mutable contexts so it doesn't really matter.

[madadam]

... can you please try this branch to see if it fixes the segfaults ya'll were seeing?

This does seem to fix the segfault for me, nice! 👍

[link2xt]

We have switched back to rusqlite for now, so I have no code to do testing: deltachat/deltachat-core-rust#2380 Could try to revert this change and rebase/update it for the latest version, but it's unlikely that deltachat-core-rust can switch to sqlx due to lower performance, which is a separate issue.

So don't wait for me to approve, if @madadam says it fixes the bug for them then it's good for merging.

[dicej]

FWIW, my app consistently segfaults with sqlx v0.5.5, and this PR seems to have fixed it. Happy to provide more info if it's useful.

Thanks to @abonander, @madadam, @link2xt and anyone else who helped address these issues!

[dicej]

Addendum to my comment above: I'm now seeing occasional panics from my build script which may indicate StatementHandles are sometimes outliving their ConnectionHandles:

error: failed to run custom build command for `tagger-server v0.1.0 (/home/dicej/p/tagger/server)`

Caused by:
  process didn't exit successfully: `/home/dicej/p/tagger/target/debug/build/tagger-server-378e4c9308a69835/build-script-build` (exit status: 101)
  --- stdout
  cargo:rustc-env=DATABASE_URL=sqlite:///home/dicej/p/tagger/server/target/schema.dat

  --- stderr
  thread 'main' panicked at 'unable to close due to unfinalized statements or unfinished backups', /home/dicej/.cargo/git/checkouts/sqlx-f05f33ba4f5c3036/b4cf1a0/sqlx-core/src/sqlite/connection/handle.rs:43:17
  stack backtrace:
     0: rust_begin_unwind
               at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:515:5
     1: std::panicking::begin_panic_fmt
               at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:457:5
     2: <sqlx_core::sqlite::connection::handle::ConnectionHandle as core::ops::drop::Drop>::drop
               at /home/dicej/.cargo/git/checkouts/sqlx-f05f33ba4f5c3036/b4cf1a0/sqlx-core/src/sqlite/connection/handle.rs:43:17
     3: core::ptr::drop_in_place<sqlx_core::sqlite::connection::handle::ConnectionHandle>
               at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/ptr/mod.rs:192:1
     4: core::ptr::drop_in_place<sqlx_core::sqlite::connection::SqliteConnection>
               at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/ptr/mod.rs:192:1
     5: build_script_build::main::{{closure}}
               at ./build.rs:32:1
     6: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
               at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/future/mod.rs:80:19
     7: tokio::park::thread::CachedParkThread::block_on::{{closure}}
               at /home/dicej/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.8.2/src/park/thread.rs:263:54
     8: tokio::coop::with_budget::{{closure}}
               at /home/dicej/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.8.2/src/coop.rs:106:9
     9: std::thread::local::LocalKey<T>::try_with
               at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/thread/local.rs:400:16
    10: std::thread::local::LocalKey<T>::with
               at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/thread/local.rs:376:9
    11: tokio::coop::with_budget
               at /home/dicej/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.8.2/src/coop.rs:99:5
    12: tokio::coop::budget
               at /home/dicej/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.8.2/src/coop.rs:76:5
    13: tokio::park::thread::CachedParkThread::block_on
               at /home/dicej/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.8.2/src/park/thread.rs:263:31
    14: tokio::runtime::enter::Enter::block_on
               at /home/dicej/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.8.2/src/runtime/enter.rs:151:13
    15: tokio::runtime::thread_pool::ThreadPool::block_on
               at /home/dicej/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.8.2/src/runtime/thread_pool/mod.rs:71:9
    16: tokio::runtime::Runtime::block_on
               at /home/dicej/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.8.2/src/runtime/mod.rs:452:43
    17: build_script_build::main
               at ./build.rs:31:5
    18: core::ops::function::FnOnce::call_once
               at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/ops/function.rs:227:5
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
warning: build failed, waiting for other jobs to finish...
error: build failed

[abonander]

Hmm, this probably means we need to refcount the database handle as well, and keep a reference to it in the StatementWorker.

What's likely happening there is the StatementWorker is in the middle of running sqlite3_reset() when the SqliteConnection is dropped which will attempt to finalize the database handle.

[abonander]

@dicej can you update and try again?

[dicej]

@dicej can you update and try again?

Yes, it's looking good now. I've run touch server/build.rs && cargo build about a dozen times with no panics.