Skip to content

Commit

Permalink
fix(ext/node): throw Session methods when database is closed (#27968)
Browse files Browse the repository at this point in the history
  • Loading branch information
littledivy authored Feb 6, 2025
1 parent ece384c commit 28faaee
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 2 deletions.
2 changes: 1 addition & 1 deletion ext/node/ops/sqlite/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ impl DatabaseSync {
Ok(Session {
inner: raw_session,
freed: Cell::new(false),
_db: self.conn.clone(),
db: self.conn.clone(),
})
}
}
12 changes: 11 additions & 1 deletion ext/node/ops/sqlite/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub struct Session {
pub(crate) freed: Cell<bool>,

// Hold a strong reference to the database.
pub(crate) _db: Rc<RefCell<Option<rusqlite::Connection>>>,
pub(crate) db: Rc<RefCell<Option<rusqlite::Connection>>>,
}

impl GarbageCollected for Session {}
Expand Down Expand Up @@ -57,6 +57,10 @@ impl Session {
// Closes the session.
#[fast]
fn close(&self) -> Result<(), SqliteError> {
if self.db.borrow().is_none() {
return Err(SqliteError::AlreadyClosed);
}

self.delete()
}

Expand All @@ -66,6 +70,9 @@ impl Session {
// This method is a wrapper around `sqlite3session_changeset()`.
#[buffer]
fn changeset(&self) -> Result<Box<[u8]>, SqliteError> {
if self.db.borrow().is_none() {
return Err(SqliteError::AlreadyClosed);
}
if self.freed.get() {
return Err(SqliteError::SessionClosed);
}
Expand All @@ -78,6 +85,9 @@ impl Session {
// This method is a wrapper around `sqlite3session_patchset()`.
#[buffer]
fn patchset(&self) -> Result<Box<[u8]>, SqliteError> {
if self.db.borrow().is_none() {
return Err(SqliteError::AlreadyClosed);
}
if self.freed.get() {
return Err(SqliteError::SessionClosed);
}
Expand Down
3 changes: 3 additions & 0 deletions tests/unit_node/sqlite_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ Deno.test("[node/sqlite] createSession and changesets", () => {
assertThrows(() => session.changeset(), Error, "Session is already closed");
// Close after close should throw.
assertThrows(() => session.close(), Error, "Session is already closed");

db.close();
assertThrows(() => session.close(), Error, "Database is already closed");
});

Deno.test("[node/sqlite] StatementSync integer too large", () => {
Expand Down

0 comments on commit 28faaee

Please sign in to comment.