search

WiseLibs / better-sqlite3

The fastest and simplest library for SQLite3 in Node.js.
MIT License
5.23k stars 390 forks source link

Change the threading mode with worker threads? #1138

Closed EvanHahn closed 5 months ago

EvanHahn commented 5 months ago

Short version: should this library recommend changing the threading mode to "serialized" when using worker threads?

Long version:

SQLite has three threading modes: "single-thread", "multi-thread", and "serialized". Only the last one is fully safe for multi-threaded use.

This library supports worker threads, yet it is compiled with the "multi-thread" option—confusingly, not totally safe for multi-threaded use. This seems like it might lead to bugs, such as database corruption.

To me, it seems like there are several possible solutions to this problem:

  1. Change the default compilation mode to "serialized" (SQLITE_THREADSAFE=1).
  2. Update the worker thread documentation to mention that you should probably compile with SQLITE_THREADSAFE=1, or assume the risks.
  3. Add some API for configuring this when you create the database.

I might be wrong about this, though!

If we decide to take action on this issue, I'm happy to do the work.

Prinzhorn commented 5 months ago

I might be wrong about this, though!

You are. Quoting the SQLite docs you've linked:

  1. Multi-thread. In this mode, SQLite can be safely used by multiple threads provided that no single database connection nor any object derived from database connection, such as a prepared statement, is used in two or more threads at the same time.

This is exactly the case here. If you're using worker threads you open a new connection in the thread. Nothing is ever shared across thread boundaries (the structured cloning algorithm wouldn't even allow passing connections or prepared statements, because JavaScript threads are mostly foolproof).

EvanHahn commented 5 months ago

This answers my question. Thank you!