GeezerVison is showing github.com — sqlite: prevent reentrant session.close() · nodejs/node@0a6f2ee
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: doc/api/sqlite.md +2-1Lines changed: 2 additions & 1 deletion - Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|
@@ -947,7 +947,8 @@ wrapper around [`sqlite3session_patchset()`][]. | 947 | 947 |
| 948 | 948 | ### `session.close()` | 949 | 949 |
| 950 | | -Closes the session. An exception is thrown if the database or the session is not open. This method is a | | 950 | +Closes the session. An exception is thrown if the database or the session is not open, | | 951 | +or if the session is currently generating a changeset or patchset. This method is a | 951 | 952 | wrapper around [`sqlite3session_delete()`][]. | 952 | 953 |
| 953 | 954 | ### `session[Symbol.dispose]()` | |
Collapse file Copy file name to clipboardExpand all lines: src/node_sqlite.cc +6Lines changed: 6 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -3921,6 +3921,10 @@ void Session::Changeset(const FunctionCallbackInfo<Value>& args) { | 3921 | 3921 | THROW_AND_RETURN_ON_BAD_STATE( | 3922 | 3922 | env, session->session_ == nullptr, "session is not open"); | 3923 | 3923 |
| | 3924 | + session->is_generating_changeset_ = true; | | 3925 | + auto changeset_guard = | | 3926 | + OnScopeLeave([&] { session->is_generating_changeset_ = false; }); | | 3927 | + | 3924 | 3928 | int nChangeset; | 3925 | 3929 | void* pChangeset; | 3926 | 3930 | int r = sqliteChangesetFunc(session->session_, &nChangeset, &pChangeset); | @@ -3946,6 +3950,8 @@ void Session::Close(const FunctionCallbackInfo<Value>& args) { | 3946 | 3950 | env, !session->database_->IsOpen(), "database is not open"); | 3947 | 3951 | THROW_AND_RETURN_ON_BAD_STATE( | 3948 | 3952 | env, session->session_ == nullptr, "session is not open"); | | 3953 | + THROW_AND_RETURN_ON_BAD_STATE( | | 3954 | + env, session->is_generating_changeset_, "session is currently in use"); | 3949 | 3955 |
| 3950 | 3956 | session->Delete(); | 3951 | 3957 | } | |
Collapse file Copy file name to clipboardExpand all lines: src/node_sqlite.h +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -361,6 +361,7 @@ class Session : public BaseObject { | 361 | 361 | void Delete(); | 362 | 362 | sqlite3_session* session_; | 363 | 363 | BaseObjectPtr<DatabaseSync> database_; // The Parent Database | | 364 | + bool is_generating_changeset_ = false; | 364 | 365 |
| 365 | 366 | friend class DatabaseSync; | 366 | 367 | }; | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-session.js +22Lines changed: 22 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -605,6 +605,28 @@ test('session.close() - closing twice', (t) => { | 605 | 605 | }); | 606 | 606 | }); | 607 | 607 |
| | 608 | +test('session.close() - while generating changes throws exception', (t) => { | | 609 | + for (const method of ['changeset', 'patchset']) { | | 610 | + const database = new DatabaseSync(':memory:'); | | 611 | + database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY, value TEXT)'); | | 612 | + | | 613 | + const session = database.createSession({ table: 'data' }); | | 614 | + database.exec("INSERT INTO data VALUES (1, 'a'), (2, 'b'), (3, 'c')"); | | 615 | + database.setAuthorizer(() => { | | 616 | + session.close(); | | 617 | + return constants.SQLITE_OK; | | 618 | + }); | | 619 | + | | 620 | + t.assert.throws(() => session[method](), { | | 621 | + code: 'ERR_INVALID_STATE', | | 622 | + message: 'session is currently in use', | | 623 | + }); | | 624 | + | | 625 | + database.setAuthorizer(null); | | 626 | + t.assert.notStrictEqual(session[method]().length, 0); | | 627 | + } | | 628 | +}); | | 629 | + | 608 | 630 | test('session - keeps its database alive after the db handle is dropped', async (t) => { | 609 | 631 | const { gcUntil, onGC } = require('../common/gc'); | 610 | 632 |
| |
|