-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Sqlite "Access violation" while simple upsert loop with transaction. #1467
Comments
@foriequal0 can you reproduce this with |
It still happens with 0.5.9. |
I can reproduce this under Linux. I'm actually investigating a similar segfault that happens in a project I'm working on, but haven't managed to reproduce it in a small test. I've run the exact same code from the first message in this issue (in a test, with
I get segfaults at two different points. The first one is like the one in the first message:
The second one is longer. I've redacted the beginning of the stack trace:
|
Hey I think I found the bug in sqlx. The sqlite connection is being opened with
So the database connection shouldn't be used simultaneously in two or more threads. I've seen that the ConnectionHandle doesn't implement the I believe the segmentation fault here happens because sqlite is using the Since the Since the method |
I think I encountered the same issue. Here is a much simpler reproduction case: #[sqlx_macros::test]
async fn successive_empty_transactions_dont_segfault() {
let mut conn = SqliteConnection::connect(":memory:").await.unwrap();
for _ in 0..10000 {
let tx = conn.begin().await.unwrap();
tx.commit().await.unwrap();
}
} Note I can only reproduce the crash with |
As a temporary solution until this bug is fixed, you can now set the sqlite connection to be opened in let mut opts = SqliteConnectOptions::from_str(url)?
.serialized(true);
Ok(SqlitePool::connect_with(opts).await?) This option setting was introduced recently and is not yet available in a release, so you'd need to pull the crate from github. For example, in your [patch.crates-io]
sqlx = { git = "https://github.com/launchbadge/sqlx", rev = "b419bf529823241a7fd598628919ab451fd7d136" } |
We probably don't need On the other hand, I'm wondering if we should just have the worker thread manage the connection directly, never calling any SQLite functions outside of it, and then we essentially interact with it like any other client/server database model, with only high-level commands being sent between threads. We're already talking about doing that more or less in #1175 since the SQLite driver in its current incarnation is so slow anyways. |
The only instance of
As I stated before, I believe the data race that causes this segfault is between
I believe this is the right approach to achieve a safe sqlite wrapper with |
Would you folks mind trying the changes I have in #1551 and see if the access violations still happen or not? |
I've run the test code from the first message 5 times using #1551 and all runs completed successfully without any segmentation fault :) (on Linux x86_64) @abonander thanks for working on this! |
I can confirm that #1551 fixes the issue for me as well 👍 |
Envorinment
Following code crashes while run.
log:
stacktrace:
It doesn't crash if I run it without the transaction.
The text was updated successfully, but these errors were encountered: