Protect critical sections better with mutexes

[warmwaffles]

As this integration has grown, we need to be careful about what is and is not protected. All calls exposed through the NIF that interact with a statement or connection, need to acquire a mutex lock before operating on the database. Nested helper function calls should refrain from locking anything, otherwise we'll wind up with a deadlock.

@ruslandoga this will probably have some conflicts with any work you are doing.

[ruslandoga]

👋 @warmwaffles

No problems from my side :) But thank you for the heads up!

---

FWIW, I went in the opposite direction in my fork and removed all mutexes. Instead, the pool manages access to the resources using processes and the NIF just exposes SQLite C API to it.

[ruslandoga]

This one is probably unnecessary. It won't help if there was an insert from another process after the latest insert and before exqlite_changes call in the current process.

[warmwaffles]

Hmm. Reading the docs it does make call out

** If a separate thread makes changes on the same database connection
** while [sqlite3_changes()] is running then the value returned
** is unpredictable and not meaningful.

I'll remove these. Side note @ruslandoga should we be using sqlite_changes64 instead of sqlite_changes here? Would be kinda crazy for someone to have updated more than 32 bits worth of an integer.

[ruslandoga]

Side note @ruslandoga should we be using sqlite_changes64 instead of sqlite_changes here?

I don't know :) I guess sqlite_changes64 is safer, but I agree about how unlikely the problem is.

Hmm. Reading the docs it does make call out

Ah, right. Then it makes sense to have this mutexed. I thought of something like this instead:

Process A:
mutex(insert ...)

Process B:
mutex(insert ...)

Process A:
mutex(changes)

Process A would still get changes from Process B's insert, even with mutexed calls.

[warmwaffles]

But FWIW, I went in the opposite direction in XQLite and removed all mutexes. Instead, the pool manages access to the resources using processes and the NIF just exposes SQLite C API to it.

While I think it is okay-ish to gate the calls via process management in erlang/elixir, I do have reservations about it. One of the concerns I have is someone using the Exqlite module directly. Standing up a connection and then handing it to a Task.Supervisor is one way I can see this potentially blowing up if gated by a single process.

One interesting thing I'd like to maybe see some benchmarks on is going through a single process that controls access vs using mutexes to access the DB. My gut says, throughput using mutexes will be faster than the single process gating.

[warmwaffles]

Closing this. I need to rework this from a different angle. I'm getting deadlocks.