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

Not safe for use concurrently from multiple async scopes #1761

Open
segevfiner opened this issue Feb 13, 2024 · 4 comments
Open

Not safe for use concurrently from multiple async scopes #1761

segevfiner opened this issue Feb 13, 2024 · 4 comments
Labels

Comments

@segevfiner
Copy link

Issue Summary

If you use a single Database from multiple different async contexts, such as handling two requests concurrently in a web server, and both use transactions, the transactions will get interleaved, meaning you might end up trying to start another transaction while one is already ongoing in the connection, which will error, or you will end up throwing in a random query into an unrelated transaction started by a different request.

To use the library safely concurrently with one Database, there needs to be a mechanism to "lock" the database while one context is performing a transaction, or any other action that effects the connection state, which will block other queries/transaction from other contexts while that lock is held. (Might be possible to achieve using the builtin db lock of SQLite3, the API level one sqlite_db_mutex, not the database level one), and of course, such a lock needs to use callbacks/async to not actually block Node.js.

serialize is not enough, as other concurrent code can still use the DB, that is, end up queueing queries.

This even tripped up TypeORM typeorm/typeorm#1884, and IMHO makes the library unusable like this without connection pooling or some other external locking mechanism.

Steps to Reproduce

Run multiple transactions concurrently, e.g. From multiple web requests.

Version

5.1.7

Node.js Version

18.19.0

How did you install the library?

pnpm 8 on macOS 14.3.1

@segevfiner segevfiner added the bug label Feb 13, 2024
@thebinarysearchtree
Copy link

thebinarysearchtree commented Feb 23, 2024

My ORM handles this properly because I actually know how sqlite works. Each transaction must have its own database connection.

https://github.com/thebinarysearchtree/flyweight#transactions-and-concurrency

@segevfiner
Copy link
Author

Qn API like https://github.com/WiseLibs/better-sqlite3/blob/master/docs/api.md#transactionfunction---function can help, by correctly locking the DB around the execution of the async callback inside.

@thebinarysearchtree
Copy link

thebinarysearchtree commented Mar 12, 2024

SQLite is designed to allow you to continue querying the database during a transaction as long as you use a different connection.

@segevfiner
Copy link
Author

Yes. I know. SQlite3 has file locking in place. Including handling for multiple connections in the same process with the weird semantics of fcntl.

But it can sometimes be more convenient to just use a single connection when concurrency isn't high, rather than a full blown connection pool, especially when using a memory database, where opening multiple connections to one is more involved.

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

2 participants