Skip to content

Commit 15c65c9

Browse files
committed
fix(core): close DB before deleting local data folder on notebook close
- Add Notebook::Close() to release MetadataStore before cleanup - Fix duplicate notebooks_.erase() causing undefined behavior - Add test for proper DB lock release on Windows
1 parent 34c48fc commit 15c65c9

File tree

4 files changed

+69
-0
lines changed

4 files changed

+69
-0
lines changed

src/core/notebook.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,19 @@ VxCoreError Notebook::InitMetadataStore() {
155155
return VXCORE_OK;
156156
}
157157

158+
void Notebook::Close() {
159+
VXCORE_LOG_INFO("Closing notebook: id=%s", config_.id.c_str());
160+
161+
// Close MetadataStore first to release DB file lock
162+
if (metadata_store_) {
163+
metadata_store_->Close();
164+
metadata_store_.reset();
165+
}
166+
167+
// Reset folder manager
168+
folder_manager_.reset();
169+
}
170+
158171
TagNode *Notebook::FindTag(const std::string &tag_name) {
159172
for (auto &tag : config_.tags) {
160173
if (tag.name == tag_name) {

src/core/notebook.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ class Notebook {
6969
FolderManager *GetFolderManager() { return folder_manager_.get(); }
7070
MetadataStore *GetMetadataStore() { return metadata_store_.get(); }
7171

72+
// Closes the notebook, releasing all resources (DB connections, etc.).
73+
// Must be called before deleting the notebook's local data folder.
74+
void Close();
75+
7276
VxCoreError CreateFolderPath(const std::string &folder_path, std::string &out_folder_id);
7377

7478
VxCoreError CreateTag(const std::string &tag_name, const std::string &parent_tag = "");

src/core/notebook_manager.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ VxCoreError NotebookManager::CloseNotebook(const std::string &notebook_id) {
180180
return VXCORE_ERR_NOT_FOUND;
181181
}
182182

183+
// Close notebook first to release DB file lock before deleting local data
184+
it->second->Close();
185+
183186
DeleteNotebookLocalData(*it->second);
184187

185188
notebooks_.erase(it);

tests/test_notebook.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,54 @@ int test_tag_create_path_trailing_slash() {
967967
return 0;
968968
}
969969

970+
// Test that closing a notebook properly releases the DB file lock
971+
// so that the local data folder can be deleted (especially important on Windows)
972+
int test_notebook_close_releases_db_lock() {
973+
std::cout << " Running test_notebook_close_releases_db_lock..." << std::endl;
974+
cleanup_test_dir("test_nb_close_db");
975+
976+
VxCoreContextHandle ctx = nullptr;
977+
VxCoreError err = vxcore_context_create(nullptr, &ctx);
978+
ASSERT_EQ(err, VXCORE_OK);
979+
980+
// Create a bundled notebook (which has a local data folder with DB)
981+
char *notebook_id = nullptr;
982+
err = vxcore_notebook_create(ctx, "test_nb_close_db", "{\"name\":\"Close DB Test\"}",
983+
VXCORE_NOTEBOOK_BUNDLED, &notebook_id);
984+
ASSERT_EQ(err, VXCORE_OK);
985+
ASSERT_NOT_NULL(notebook_id);
986+
987+
// Create some content to ensure DB is used
988+
char *folder_id = nullptr;
989+
err = vxcore_folder_create(ctx, notebook_id, ".", "docs", &folder_id);
990+
ASSERT_EQ(err, VXCORE_OK);
991+
vxcore_string_free(folder_id);
992+
993+
char *file_id = nullptr;
994+
err = vxcore_file_create(ctx, notebook_id, "docs", "test.md", &file_id);
995+
ASSERT_EQ(err, VXCORE_OK);
996+
vxcore_string_free(file_id);
997+
998+
// Close the notebook - this should release the DB lock and delete local data
999+
err = vxcore_notebook_close(ctx, notebook_id);
1000+
ASSERT_EQ(err, VXCORE_OK);
1001+
1002+
// Verify the notebook is no longer accessible
1003+
char *config_json = nullptr;
1004+
err = vxcore_notebook_get_config(ctx, notebook_id, &config_json);
1005+
ASSERT_EQ(err, VXCORE_ERR_NOT_FOUND);
1006+
1007+
// The notebook root folder should still exist (it's not deleted on close)
1008+
ASSERT(path_exists("test_nb_close_db"));
1009+
ASSERT(path_exists("test_nb_close_db/vx_notebook/config.json"));
1010+
1011+
vxcore_string_free(notebook_id);
1012+
vxcore_context_destroy(ctx);
1013+
cleanup_test_dir("test_nb_close_db");
1014+
std::cout << " ✓ test_notebook_close_releases_db_lock passed" << std::endl;
1015+
return 0;
1016+
}
1017+
9701018
int main() {
9711019
std::cout << "Running notebook tests..." << std::endl;
9721020

@@ -975,6 +1023,7 @@ int main() {
9751023
RUN_TEST(test_notebook_create_bundled);
9761024
RUN_TEST(test_notebook_create_raw);
9771025
RUN_TEST(test_notebook_open_close);
1026+
RUN_TEST(test_notebook_close_releases_db_lock);
9781027
RUN_TEST(test_notebook_get_properties);
9791028
RUN_TEST(test_notebook_set_properties);
9801029
RUN_TEST(test_notebook_list);

0 commit comments

Comments
 (0)