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

sqlite3, check_same_thread should be true? #128697

Closed
NewUserHa opened this issue Jan 10, 2025 · 6 comments
Closed

sqlite3, check_same_thread should be true? #128697

NewUserHa opened this issue Jan 10, 2025 · 6 comments
Labels
docs Documentation in the Doc dir topic-sqlite3

Comments

@NewUserHa
Copy link
Contributor

Documentation

https://docs.python.org/3/library/sqlite3.html#module-functions:

check_same_thread (bool) – If True (default), ProgrammingError will be raised if the database connection is used by a thread other than the one that created it. If False, the connection may be accessed in multiple threads; write operations may need to be serialized by the user to avoid data corruption. See threadsafety for more information.

and

Serialized: In serialized mode, SQLite can be safely used by multiple threads with no restriction.

#71300:

... The serialized mode is default on both Mac and Windows so we can probably skip validating that. I did like mentioning the user needs to serialize the writes. They could use one thread for writing only or use locking. So, I just said to serialize.

the code:

int pysqlite_check_thread(pysqlite_Connection* self)
{
if (self->check_same_thread) {
if (PyThread_get_thread_ident() != self->thread_ident) {
PyErr_Format(self->ProgrammingError,
"SQLite objects created in a thread can only be used in that same thread. "
"The object was created in thread id %lu and this is thread id %lu.",
self->thread_ident, PyThread_get_thread_ident());
return 0;
}
}
return 1;
}
static PyObject* pysqlite_connection_get_isolation_level(pysqlite_Connection* self, void* unused)

the issue:
the sqlite originally itself does not have this check_same_thread option, and
https://sqlite.org/c3ref/c_config_covering_index_scan.html#sqliteconfigserialized:

SQLITE_CONFIG_SERIALIZED
There are no arguments to this option. This option sets the threading mode to Serialized. In other words, this option enables all mutexes including the recursive mutexes on database connection and prepared statement objects. In this mode (which is the default when SQLite is compiled with SQLITE_THREADSAFE=1) the SQLite library will itself serialize access to database connections and prepared statements so that the application is free to use the same database connection or the same prepared statement in different threads at the same time. If SQLite is compiled with the SQLITE_THREADSAFE=0 compile-time option then it is not possible to set the Serialized threading mode and sqlite3_config() will return SQLITE_ERROR if called with the SQLITE_CONFIG_SERIALIZED configuration option.

according sqlite itself said the SQLite library will itself serialize access to database connections and prepared statements so that the application is free to use the same database connection or the same prepared statement in different threads at the same time., why does python sqlite3 need this check_same_thread option? and the sqlite3.threadsafety returned also is 3
the document may statement clear it

@NewUserHa NewUserHa added the docs Documentation in the Doc dir label Jan 10, 2025
@erlend-aasland
Copy link
Contributor

IIUC, you want check_same_thread to be set dynamically? That would be a breaking change; we're not going to do that.

Moreover, if the underlying sqlite3 * database pointer is thread-safe, that does not imply our pysqlite_Connection wrapper is also thread-safe. You may still have to lock in order to prevent races on the connection object itself (the Python object, not the SQLite database pointer).

@erlend-aasland erlend-aasland closed this as not planned Won't fix, can't repro, duplicate, stale Jan 10, 2025
@NewUserHa
Copy link
Contributor Author

the returned value "sqlite3.threadsafety" is 3, which is Serialized
the document: "Serialized: In serialized mode, SQLite can be safely used by multiple threads with no restriction.",

if the python itself does not actually support Serialized of sqlite, the document should be updated to clear with it with "check_same_thread since currently pysqlite_Connection is not thread-safe"

furthermore, python's pysqlite_Connection wrapper should update to actually utilize sqlite's thread-safety, it can be used in case like asyncio.run_in_executer()

@erlend-aasland
Copy link
Contributor

In the docs, we consistently use sqlite3 for the stdlib module and SQLite for the underlying library.

@NewUserHa
Copy link
Contributor Author

NewUserHa commented Jan 10, 2025

or at least use "sqlite3.threadsafety" to indicate it "it is not 3(Serialized), and python sqlite3 does not support Serialized", otherwise it creates conflict on meaning, and creates confuses

@NewUserHa
Copy link
Contributor Author

or just drop "sqlite3.threadsafety" if that python sqlite3 has nothing to do with it

@erlend-aasland
Copy link
Contributor

or just drop "sqlite3.threadsafety" if that python sqlite3 has nothing to do with it

The .threadsafety API is required by PEP-249; we cannot just remove it. That would also be a breaking change, which we want to avoid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir topic-sqlite3
Projects
Status: Todo
Development

No branches or pull requests

3 participants