From 4d126d3aa0aa2c7baf023cb5a7fb934aee24a8dd Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 2 Feb 2024 08:28:57 -0500 Subject: [PATCH] Fix enough bugs to run a simple test Signed-off-by: Cole Miller --- src/vfs2.c | 48 ++++++++++++++++------- test/unit/test_vfs2.c | 90 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 13 deletions(-) diff --git a/src/vfs2.c b/src/vfs2.c index 92a336545..cf1309189 100644 --- a/src/vfs2.c +++ b/src/vfs2.c @@ -109,6 +109,8 @@ struct vfs2_wal dqlite_vfs_frame *pending_txn_frames; /* for vfs2_poll */ uint32_t pending_txn_last_frame_commit; /* commit marker for the physical last frame */ + + bool is_empty; }; struct vfs2_db @@ -342,6 +344,15 @@ static int vfs2_wal_swap(struct vfs2_file *wal, const struct vfs2_wal_hdr *hdr) assert(hdr->salt[1] == region0->hdr.basic[0].aCksum[1]); db->db_shm.prev_txn_hdr = region0->hdr.basic[0]; + dqlite_vfs_frame *frames = wal->wal.pending_txn_frames; + uint32_t n = wal->wal.pending_txn_len; + for (uint32_t i = 0; i < n; i++) { + sqlite3_free(frames[i].data); + } + sqlite3_free(frames); + wal->wal.pending_txn_frames = NULL; + wal->wal.pending_txn_len = 0; + /* Move the moving name. */ rv = unlink(wal->wal.moving_name); if (rv != 0) { @@ -353,7 +364,7 @@ static int vfs2_wal_swap(struct vfs2_file *wal, const struct vfs2_wal_hdr *hdr) } (void)phys_outgoing->pMethods->xWrite(phys_outgoing, &invalid_magic, - sizeof(invalid_magic), 0); + sizeof(invalid_magic), 0); return SQLITE_OK; } @@ -369,12 +380,10 @@ static int vfs2_wal_write_frame_hdr(struct vfs2_file *wal, /* Appending to the end of the WAL. */ if (wal->wal.pending_txn_last_frame_commit > 0) { /* Start of a new transaction. */ - if (frames != NULL) { - for (uint32_t i = 0; i < n; i++) { - sqlite3_free(frames[i].data); - } - sqlite3_free(frames); + for (uint32_t i = 0; i < n; i++) { + sqlite3_free(frames[i].data); } + sqlite3_free(frames); wal->wal.pending_txn_frames = NULL; wal->wal.pending_txn_start += n; wal->wal.pending_txn_len = 0; @@ -386,7 +395,7 @@ static int vfs2_wal_write_frame_hdr(struct vfs2_file *wal, if (wal->wal.pending_txn_frames == NULL) { return SQLITE_NOMEM; } - dqlite_vfs_frame *frame = &frames[n]; + dqlite_vfs_frame *frame = &wal->wal.pending_txn_frames[n]; frame->page_number = ByteGetBe32(buf); frame->data = NULL; wal->wal.pending_txn_last_frame_commit = @@ -417,8 +426,8 @@ static int vfs2_wal_post_write(struct vfs2_file *wal, x /= frame_size; return vfs2_wal_write_frame_hdr(wal, buf, x); } else if (amt == (int)wal->vfs_data->page_size) { - sqlite3_int64 x = - ofst - VFS2_WAL_FRAME_HDR_SIZE - sizeof(struct vfs2_wal_hdr); + sqlite3_int64 x = ofst - VFS2_WAL_FRAME_HDR_SIZE - + sizeof(struct vfs2_wal_hdr); assert(x % frame_size == 0); x /= frame_size; x -= wal->wal.pending_txn_start; @@ -444,7 +453,11 @@ static int vfs2_write(sqlite3_file *file, if ((xfile->flags & SQLITE_OPEN_WAL) && ofst == 0) { assert(amt == sizeof(struct vfs2_wal_hdr)); - return vfs2_wal_swap(xfile, buf); + bool is_empty = xfile->wal.is_empty; + xfile->wal.is_empty = false; + if (!is_empty) { + return vfs2_wal_swap(xfile, buf); + } } rv = xfile->orig->pMethods->xWrite(xfile->orig, buf, amt, ofst); @@ -719,7 +732,11 @@ static struct sqlite3_io_methods vfs2_io_methods = { /* sqlite3_vfs implementations begin here */ -static int compare_wals(sqlite3_file *xwal1, sqlite3_file *xwal2, bool *invert) { +static int compare_wals(sqlite3_file *xwal1, + sqlite3_file *xwal2, + bool *invert, + bool *empty) +{ int rv; sqlite3_int64 size1; rv = xwal1->pMethods->xFileSize(xwal1, &size1); @@ -732,6 +749,10 @@ static int compare_wals(sqlite3_file *xwal1, sqlite3_file *xwal2, bool *invert) return rv; } + if (size1 == 0 && size2 == 0) { + *empty = true; + } + if (size2 == 0) { *invert = false; } else if (size1 == 0) { @@ -758,7 +779,6 @@ static int compare_wals(sqlite3_file *xwal1, sqlite3_file *xwal2, bool *invert) } } - return SQLITE_OK; } @@ -812,7 +832,7 @@ static int vfs2_open_wal(sqlite3_vfs *vfs, } bool invert; - rv = compare_wals(phys1, phys2, &invert); + rv = compare_wals(phys1, phys2, &invert, &xout->wal.is_empty); if (invert) { rv = unlink(name); (void)rv; @@ -845,6 +865,8 @@ static int vfs2_open_wal(sqlite3_vfs *vfs, } } + xout->wal.is_empty = true; + register_file(xout, &entry); sqlite3_free(entry); return SQLITE_OK; diff --git a/test/unit/test_vfs2.c b/test/unit/test_vfs2.c index 63c0b804f..ad037fe66 100644 --- a/test/unit/test_vfs2.c +++ b/test/unit/test_vfs2.c @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -373,6 +374,95 @@ TEST(vfs2_open, tmp, setUp, tearDown, 0, NULL) return MUNIT_OK; } +/* Various edge cases of opening a WAL */ +TEST(vfs2_open, wal_weirdness, setUp, tearDown, 0, NULL) +{ + (void)params; + int rc; + struct fixture *f = data; + sqlite3_file *db = munit_malloc(f->vfs->szOsFile); + sqlite3_file *wal = munit_malloc(f->vfs->szOsFile); + int flags; + + vfsFillPath(f, "foo.db"); + rc = f->vfs->xOpen( + f->vfs, f->path, db, + SQLITE_OPEN_CREATE | SQLITE_OPEN_READWRITE | SQLITE_OPEN_MAIN_DB, + &flags); + munit_assert_int(rc, ==, SQLITE_OK); + + vfsFillPath(f, "foo.db-wal"); + rc = f->vfs->xOpen( + f->vfs, f->path, wal, + SQLITE_OPEN_CREATE | SQLITE_OPEN_READWRITE | SQLITE_OPEN_WAL, + &flags); + munit_assert_int(rc, ==, SQLITE_OK); + + /* Now the two physical WALs and the moving name exist. The moving name + * points to -xwal1. */ + struct stat st; + rc = stat(f->path, &st); + munit_assert_int(rc, ==, 0); + unsigned long long ino = st.st_ino; + char fixed1[VFS_PATH_SZ]; + snprintf(fixed1, VFS_PATH_SZ, "%s/%s", f->dir, "foo.db-xwal1"); + rc = stat(fixed1, &st); + munit_assert_int(rc, ==, 0); + unsigned long long ino1 = st.st_ino; + munit_assert_ullong(ino, ==, ino1); + munit_assert_ullong(st.st_size, ==, 0); + char fixed2[VFS_PATH_SZ]; + snprintf(fixed2, VFS_PATH_SZ, "%s/%s", f->dir, "foo.db-xwal2"); + rc = stat(fixed2, &st); + munit_assert_int(rc, ==, 0); + munit_assert_ullong(st.st_size, ==, 0); + unsigned long ino2 = st.st_ino; + + /* Write the header for the first time. This goes to xwal1. The moving + * name doesn't move. */ + char hdr[32] = {0}; + rc = wal->pMethods->xWrite(wal, hdr, sizeof(hdr), 0); + munit_assert_int(rc, ==, SQLITE_OK); + rc = stat(fixed1, &st); + munit_assert_ullong(st.st_size, ==, sizeof(hdr)); + rc = stat(f->path, &st); + munit_assert_int(rc, ==, 0); + munit_assert_ullong(st.st_ino, ==, ino1); + + char buf[24 + 512]; + /* Write a frame to the WAL. */ + rc = wal->pMethods->xWrite(wal, buf, 24, 32); + munit_assert_int(rc, ==, 0); + rc = wal->pMethods->xWrite(wal, buf + 24, 512, 32 + 24); + munit_assert_int(rc, ==, 0); + + /* Force allocation of the WAL index. */ + volatile void *p; + rc = db->pMethods->xShmMap(db, 0, 1 << 15, 1, &p); + munit_assert_int(rc, ==, 0); + + /* Write the header again. This triggers a real WAL swap. */ + rc = wal->pMethods->xWrite(wal, hdr, sizeof(hdr), 0); + munit_assert_int(rc, ==, SQLITE_OK); + rc = stat(fixed1, &st); + munit_assert_int(rc, ==, 0); + munit_assert_ullong(st.st_size, ==, sizeof(hdr) + sizeof(buf)); + rc = stat(fixed2, &st); + munit_assert_int(rc, ==, 0); + munit_assert_ullong(st.st_size, ==, sizeof(hdr)); + rc = stat(f->path, &st); + munit_assert_int(rc, ==, 0); + munit_assert_ullong(st.st_ino, ==, ino2); + + wal->pMethods->xClose(wal); + db->pMethods->xClose(db); + + free(wal); + free(db); + + return MUNIT_OK; +} + /****************************************************************************** * * xDelete