From f47839b23ba532118ca7459c96fa1fccd566dd8f Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sat, 22 Nov 2025 01:17:01 +0900 Subject: [PATCH 1/7] Fix sqlite connection reinitialization --- crates/stdlib/src/sqlite.rs | 73 +++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/crates/stdlib/src/sqlite.rs b/crates/stdlib/src/sqlite.rs index e19aca1f6d5..9606463e403 100644 --- a/crates/stdlib/src/sqlite.rs +++ b/crates/stdlib/src/sqlite.rs @@ -833,10 +833,10 @@ mod _sqlite { #[derive(PyPayload)] struct Connection { db: PyMutex>, - detect_types: c_int, + detect_types: PyAtomic, isolation_level: PyAtomicRef>, - check_same_thread: bool, - thread_ident: ThreadId, + check_same_thread: PyAtomic, + thread_ident: PyMutex, row_factory: PyAtomicRef>, text_factory: PyAtomicRef, } @@ -867,10 +867,10 @@ mod _sqlite { let conn = Self { db: PyMutex::new(db), - detect_types: args.detect_types, + detect_types: Radium::new(args.detect_types), isolation_level: PyAtomicRef::from(args.isolation_level), - check_same_thread: args.check_same_thread, - thread_ident: std::thread::current().id(), + check_same_thread: Radium::new(args.check_same_thread), + thread_ident: PyMutex::new(std::thread::current().id()), row_factory: PyAtomicRef::from(None), text_factory: PyAtomicRef::from(text_factory), }; @@ -899,13 +899,35 @@ mod _sqlite { type Args = ConnectArgs; fn init(zelf: PyRef, args: Self::Args, vm: &VirtualMachine) -> PyResult<()> { - let mut guard = zelf.db.lock(); - if guard.is_some() { - // Already initialized - return Ok(()); + { + // Always drop the current database handle so __init__ can fully reconfigure it. + let mut guard = zelf.db.lock(); + guard.take(); } + // Reset factories to their defaults, matching CPython's behavior. + let default_text_factory = PyStr::class(&vm.ctx).to_owned().into_object(); + let _ = unsafe { zelf.row_factory.swap(None) }; + let _ = unsafe { zelf.text_factory.swap(default_text_factory) }; + + // Attempt to open the new database before mutating other state so failures leave + // the connection uninitialized (and subsequent operations raise ProgrammingError). let db = Self::initialize_db(&args, vm)?; + + let ConnectArgs { + detect_types, + isolation_level, + check_same_thread, + .. + } = args; + + zelf.detect_types.store(detect_types, Ordering::Relaxed); + zelf.check_same_thread + .store(check_same_thread, Ordering::Relaxed); + *zelf.thread_ident.lock() = std::thread::current().id(); + let _ = unsafe { zelf.isolation_level.swap(isolation_level) }; + + let mut guard = zelf.db.lock(); *guard = Some(db); Ok(()) } @@ -1446,15 +1468,17 @@ mod _sqlite { } fn check_thread(&self, vm: &VirtualMachine) -> PyResult<()> { - if self.check_same_thread && (std::thread::current().id() != self.thread_ident) { - Err(new_programming_error( - vm, - "SQLite objects created in a thread can only be used in that same thread." - .to_owned(), - )) - } else { - Ok(()) + if self.check_same_thread.load(Ordering::Relaxed) { + let creator_id = *self.thread_ident.lock(); + if std::thread::current().id() != creator_id { + return Err(new_programming_error( + vm, + "SQLite objects created in a thread can only be used in that same thread." + .to_owned(), + )); + } } + Ok(()) } #[pygetset] @@ -1628,7 +1652,8 @@ mod _sqlite { inner.row_cast_map = zelf.build_row_cast_map(&st, vm)?; - inner.description = st.columns_description(zelf.connection.detect_types, vm)?; + let detect_types = zelf.connection.detect_types.load(Ordering::Relaxed); + inner.description = st.columns_description(detect_types, vm)?; if ret == SQLITE_ROW { drop(st); @@ -1676,7 +1701,8 @@ mod _sqlite { )); } - inner.description = st.columns_description(zelf.connection.detect_types, vm)?; + let detect_types = zelf.connection.detect_types.load(Ordering::Relaxed); + inner.description = st.columns_description(detect_types, vm)?; inner.rowcount = if stmt.is_dml { 0 } else { -1 }; @@ -1841,7 +1867,8 @@ mod _sqlite { st: &SqliteStatementRaw, vm: &VirtualMachine, ) -> PyResult>> { - if self.connection.detect_types == 0 { + let detect_types = self.connection.detect_types.load(Ordering::Relaxed); + if detect_types == 0 { return Ok(vec![]); } @@ -1849,7 +1876,7 @@ mod _sqlite { let num_cols = st.column_count(); for i in 0..num_cols { - if self.connection.detect_types & PARSE_COLNAMES != 0 { + if detect_types & PARSE_COLNAMES != 0 { let col_name = st.column_name(i); let col_name = ptr_to_str(col_name, vm)?; let col_name = col_name @@ -1864,7 +1891,7 @@ mod _sqlite { continue; } } - if self.connection.detect_types & PARSE_DECLTYPES != 0 { + if detect_types & PARSE_DECLTYPES != 0 { let decltype = st.column_decltype(i); let decltype = ptr_to_str(decltype, vm)?; if let Some(decltype) = decltype.split_terminator(&[' ', '(']).next() { From b3d45aebac5a2bac5757be78245d0adcf431d6b6 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sat, 22 Nov 2025 01:28:38 +0900 Subject: [PATCH 2/7] Align sqlite connection reinit with CPython --- crates/stdlib/src/sqlite.rs | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/stdlib/src/sqlite.rs b/crates/stdlib/src/sqlite.rs index 9606463e403..c06cbd86ee1 100644 --- a/crates/stdlib/src/sqlite.rs +++ b/crates/stdlib/src/sqlite.rs @@ -833,6 +833,7 @@ mod _sqlite { #[derive(PyPayload)] struct Connection { db: PyMutex>, + initialized: PyAtomic, detect_types: PyAtomic, isolation_level: PyAtomicRef>, check_same_thread: PyAtomic, @@ -865,8 +866,11 @@ mod _sqlite { None }; + let initialized = db.is_some(); + let conn = Self { db: PyMutex::new(db), + initialized: Radium::new(initialized), detect_types: Radium::new(args.detect_types), isolation_level: PyAtomicRef::from(args.isolation_level), check_same_thread: Radium::new(args.check_same_thread), @@ -899,16 +903,14 @@ mod _sqlite { type Args = ConnectArgs; fn init(zelf: PyRef, args: Self::Args, vm: &VirtualMachine) -> PyResult<()> { - { - // Always drop the current database handle so __init__ can fully reconfigure it. - let mut guard = zelf.db.lock(); - guard.take(); - } + let was_initialized = zelf.initialized.swap(false, Ordering::Relaxed); // Reset factories to their defaults, matching CPython's behavior. - let default_text_factory = PyStr::class(&vm.ctx).to_owned().into_object(); - let _ = unsafe { zelf.row_factory.swap(None) }; - let _ = unsafe { zelf.text_factory.swap(default_text_factory) }; + zelf.reset_factories(vm); + + if was_initialized { + zelf.drop_db(); + } // Attempt to open the new database before mutating other state so failures leave // the connection uninitialized (and subsequent operations raise ProgrammingError). @@ -929,12 +931,23 @@ mod _sqlite { let mut guard = zelf.db.lock(); *guard = Some(db); + zelf.initialized.store(true, Ordering::Relaxed); Ok(()) } } #[pyclass(with(Constructor, Callable, Initializer), flags(BASETYPE))] impl Connection { + fn drop_db(&self) { + self.db.lock().take(); + } + + fn reset_factories(&self, vm: &VirtualMachine) { + let default_text_factory = PyStr::class(&vm.ctx).to_owned().into_object(); + let _ = unsafe { self.row_factory.swap(None) }; + let _ = unsafe { self.text_factory.swap(default_text_factory) }; + } + fn initialize_db(args: &ConnectArgs, vm: &VirtualMachine) -> PyResult { let path = args.database.to_cstring(vm)?; let db = Sqlite::from(SqliteRaw::open(path.as_ptr(), args.uri, vm)?); @@ -1025,7 +1038,7 @@ mod _sqlite { #[pymethod] fn close(&self, vm: &VirtualMachine) -> PyResult<()> { self.check_thread(vm)?; - self.db.lock().take(); + self.drop_db(); Ok(()) } From 216ab899806ccaf4ea06c37116bb1e4793583523 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sat, 22 Nov 2025 01:28:48 +0900 Subject: [PATCH 3/7] Enable sqlite test_connection_bad_reinit --- Lib/test/test_sqlite3/test_dbapi.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_sqlite3/test_dbapi.py b/Lib/test/test_sqlite3/test_dbapi.py index 49c9764a1b7..2af9bfb87f0 100644 --- a/Lib/test/test_sqlite3/test_dbapi.py +++ b/Lib/test/test_sqlite3/test_dbapi.py @@ -573,8 +573,6 @@ def test_connection_reinit(self): self.assertTrue(all(isinstance(r, sqlite.Row) for r in rows)) self.assertEqual([r[0] for r in rows], ["2", "3"]) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_connection_bad_reinit(self): cx = sqlite.connect(":memory:") with cx: From 5150186e33db0d20cf9483b69d61a581b931ac6f Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sat, 22 Nov 2025 01:38:02 +0900 Subject: [PATCH 4/7] Fix sqlite reinit flag without threading --- crates/stdlib/src/sqlite.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/stdlib/src/sqlite.rs b/crates/stdlib/src/sqlite.rs index c06cbd86ee1..6b9a219807e 100644 --- a/crates/stdlib/src/sqlite.rs +++ b/crates/stdlib/src/sqlite.rs @@ -903,7 +903,7 @@ mod _sqlite { type Args = ConnectArgs; fn init(zelf: PyRef, args: Self::Args, vm: &VirtualMachine) -> PyResult<()> { - let was_initialized = zelf.initialized.swap(false, Ordering::Relaxed); + let was_initialized = Radium::swap(&zelf.initialized, false, Ordering::Relaxed); // Reset factories to their defaults, matching CPython's behavior. zelf.reset_factories(vm); @@ -931,7 +931,7 @@ mod _sqlite { let mut guard = zelf.db.lock(); *guard = Some(db); - zelf.initialized.store(true, Ordering::Relaxed); + Radium::store(&zelf.initialized, true, Ordering::Relaxed); Ok(()) } } From b2f3adf59d5b2f82fe55ff79b5d945de1edcfae4 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Sat, 22 Nov 2025 20:54:31 +0900 Subject: [PATCH 5/7] Update crates/stdlib/src/sqlite.rs --- crates/stdlib/src/sqlite.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stdlib/src/sqlite.rs b/crates/stdlib/src/sqlite.rs index 6b9a219807e..933cb990ea0 100644 --- a/crates/stdlib/src/sqlite.rs +++ b/crates/stdlib/src/sqlite.rs @@ -837,7 +837,7 @@ mod _sqlite { detect_types: PyAtomic, isolation_level: PyAtomicRef>, check_same_thread: PyAtomic, - thread_ident: PyMutex, + thread_ident: PyMutex, // TODO: Use atomic row_factory: PyAtomicRef>, text_factory: PyAtomicRef, } From ece048d0be2079e2f9a808c634f744769dbb1d17 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sat, 22 Nov 2025 21:03:14 +0900 Subject: [PATCH 6/7] Use stronger memory ordering for initialized flag synchronization Apply code review feedback to use Ordering::AcqRel for the swap operation and Ordering::Release for the store operation to ensure proper synchronization across threads when setting the initialized flag. --- crates/stdlib/src/sqlite.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/stdlib/src/sqlite.rs b/crates/stdlib/src/sqlite.rs index 933cb990ea0..e42134064f1 100644 --- a/crates/stdlib/src/sqlite.rs +++ b/crates/stdlib/src/sqlite.rs @@ -903,7 +903,7 @@ mod _sqlite { type Args = ConnectArgs; fn init(zelf: PyRef, args: Self::Args, vm: &VirtualMachine) -> PyResult<()> { - let was_initialized = Radium::swap(&zelf.initialized, false, Ordering::Relaxed); + let was_initialized = Radium::swap(&zelf.initialized, false, Ordering::AcqRel); // Reset factories to their defaults, matching CPython's behavior. zelf.reset_factories(vm); @@ -931,7 +931,7 @@ mod _sqlite { let mut guard = zelf.db.lock(); *guard = Some(db); - Radium::store(&zelf.initialized, true, Ordering::Relaxed); + Radium::store(&zelf.initialized, true, Ordering::Release); Ok(()) } } From c9fc8a7cc0fe3ab5e1426dec8aba0fe83a424ad3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 22 Nov 2025 12:32:34 +0000 Subject: [PATCH 7/7] Auto-format code [skip ci] --- crates/stdlib/src/sqlite.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stdlib/src/sqlite.rs b/crates/stdlib/src/sqlite.rs index e42134064f1..136858a769d 100644 --- a/crates/stdlib/src/sqlite.rs +++ b/crates/stdlib/src/sqlite.rs @@ -837,7 +837,7 @@ mod _sqlite { detect_types: PyAtomic, isolation_level: PyAtomicRef>, check_same_thread: PyAtomic, - thread_ident: PyMutex, // TODO: Use atomic + thread_ident: PyMutex, // TODO: Use atomic row_factory: PyAtomicRef>, text_factory: PyAtomicRef, }