From e682060e0eba9596b0a50bc7d1a960ed1527cbda Mon Sep 17 00:00:00 2001 From: dev9-bb Date: Tue, 20 Oct 2026 09:31:16 +0800 Subject: [PATCH 1/7] fix(storage): stage local objects beside their final paths --- common/block-backend-fs.c | 50 ++++--- common/obj-backend-fs.c | 141 ++++++++++++------ fileserver/objstore/backend_fs.go | 40 +++-- .../objstore/cf_local_atomic_linux_test.go | 56 +++++++ fileserver/objstore/cf_local_atomic_test.go | 116 ++++++++++++++ fileserver/objstore/cf_v2_storage.go | 9 +- tests/local_storage/block.c | 26 ++++ tests/local_storage/object.c | 21 +++ tests/local_storage/run.sh | 16 ++ tests/local_storage/stubs.c | 20 +++ 10 files changed, 410 insertions(+), 85 deletions(-) create mode 100644 fileserver/objstore/cf_local_atomic_linux_test.go create mode 100644 fileserver/objstore/cf_local_atomic_test.go create mode 100644 tests/local_storage/block.c create mode 100644 tests/local_storage/object.c create mode 100644 tests/local_storage/run.sh create mode 100644 tests/local_storage/stubs.c diff --git a/common/block-backend-fs.c b/common/block-backend-fs.c index 2d4a5b35..d056dac1 100644 --- a/common/block-backend-fs.c +++ b/common/block-backend-fs.c @@ -31,8 +31,6 @@ struct _BHandle { typedef struct { char *block_dir; int block_dir_len; - char *tmp_dir; - int tmp_dir_len; } FsPriv; static char * @@ -46,6 +44,7 @@ static int open_tmp_file (BlockBackend *bend, const char *basename, const char *store_id, + int version, char **path); static BHandle * @@ -74,7 +73,7 @@ block_backend_fs_open_block (BlockBackend *bend, return NULL; } } else { - fd = open_tmp_file (bend, block_id, store_id, &tmp_file); + fd = open_tmp_file (bend, block_id, store_id, version, &tmp_file); if (fd < 0) { ccnet_warning ("[block bend] failed to open block %s for write: %s\n", block_id, strerror(errno)); @@ -302,6 +301,10 @@ block_backend_fs_foreach_block (BlockBackend *bend, pos = path + dir_len; while ((dname1 = g_dir_read_name(dir1)) != NULL) { + /* Reserved staging files/directories are not hexadecimal shards. */ + if (strlen (dname1) != 2 || !g_ascii_isxdigit (dname1[0]) || + !g_ascii_isxdigit (dname1[1])) + continue; snprintf (pos, sizeof(path) - dir_len, "/%s", dname1); dir2 = g_dir_open (path, 0, NULL); @@ -311,7 +314,13 @@ block_backend_fs_foreach_block (BlockBackend *bend, } while ((dname2 = g_dir_read_name(dir2)) != NULL) { + /* Never enumerate partial writes (including older suffix-based + * temp names) as blocks while a writer or cold GC is active. */ + if (strlen (dname2) != 38) + continue; snprintf (block_id, sizeof(block_id), "%s%s", dname1, dname2); + if (!is_object_id_valid (block_id)) + continue; if (!process (store_id, version, block_id, user_data)) { g_dir_close (dir2); goto out; @@ -422,6 +431,10 @@ block_backend_fs_remove_store (BlockBackend *bend, GList *file_list = NULL; GList *ptr2; while ((dname2 = g_dir_read_name(dir2)) != NULL) { + /* Whole-store removal must not unlink an in-flight new-format + * staging file; a nonempty shard makes removal fail visibly. */ + if (g_str_has_prefix (dname2, ".cf-tmp-")) + continue; file_list = g_list_prepend(file_list, g_strdup(dname2)); } g_dir_close (dir2); @@ -479,20 +492,23 @@ static int open_tmp_file (BlockBackend *bend, const char *basename, const char *store_id, + int version, char **path) { - FsPriv *priv = bend->be_priv; + char final_path[SEAF_PATH_MAX]; + char *dir; int fd; - char *dir = NULL; - if (cf_v2_storage_enabled ()) { - dir = cf_v2_store_dir (priv->block_dir, store_id); - if (!dir || g_mkdir_with_parents (dir, 0700) < 0) { - g_free (dir); - return -1; - } + /* Resolve the actual final shard first. A library root or shared tmpfiles + * directory may be a different bind mount even on the same filesystem. */ + if (!get_block_path (bend, basename, final_path, store_id, version)) + return -1; + dir = g_path_get_dirname (final_path); + if (g_mkdir_with_parents (dir, 0700) < 0) { + g_free (dir); + return -1; } - *path = g_strdup_printf ("%s/%s.XXXXXX", dir ? dir : priv->tmp_dir, basename); + *path = g_strdup_printf ("%s/.cf-tmp-%s-XXXXXX", dir, basename); g_free (dir); fd = g_mkstemp (*path); if (fd < 0) @@ -514,8 +530,9 @@ block_backend_fs_new (const char *seaf_dir, const char *tmp_dir) priv->block_dir = g_build_filename (seaf_dir, "storage", "blocks", NULL); priv->block_dir_len = strlen (priv->block_dir); - priv->tmp_dir = g_strdup (tmp_dir); - priv->tmp_dir_len = strlen (tmp_dir); + /* Keep the native constructor ABI; object writes no longer depend on + * this legacy staging directory or any operational symlink beneath it. */ + (void)tmp_dir; if (g_mkdir_with_parents (priv->block_dir, 0777) < 0) { seaf_warning ("Block dir %s does not exist and" @@ -523,11 +540,6 @@ block_backend_fs_new (const char *seaf_dir, const char *tmp_dir) goto onerror; } - if (g_mkdir_with_parents (tmp_dir, 0777) < 0) { - seaf_warning ("Blocks tmp dir %s does not exist and" - " is unable to create\n", tmp_dir); - goto onerror; - } bend->open_block = block_backend_fs_open_block; bend->read_block = block_backend_fs_read_block; diff --git a/common/obj-backend-fs.c b/common/obj-backend-fs.c index 163d6e2f..9383d50d 100644 --- a/common/obj-backend-fs.c +++ b/common/obj-backend-fs.c @@ -161,6 +161,7 @@ rename_and_sync (const char *tmp_path, const char *obj_path) int dir_fd = open (parent_dir, O_RDONLY); if (dir_fd < 0) { seaf_warning ("Failed to open dir %s: %s.\n", parent_dir, strerror(errno)); + ret = -1; goto out; } @@ -217,45 +218,45 @@ rename_and_sync (const char *tmp_path, const char *obj_path) static int save_obj_contents (const char *path, const void *data, int len, gboolean need_sync) { - char tmp_path[SEAF_PATH_MAX]; - int fd; - - snprintf (tmp_path, SEAF_PATH_MAX, "%s.XXXXXX", path); + char *parent = g_path_get_dirname (path); + char *name = g_path_get_basename (path); + char *tmp_path = g_strdup_printf ("%s/.cf-tmp-%s-XXXXXX", parent, name); + int fd = -1, ret = -1, close_ret, saved_errno; + g_free (parent); + g_free (name); + + /* Keep the existing same-directory metadata publication and need_sync + * behavior, but use the reserved prefix and clean every failure path. */ fd = g_mkstemp (tmp_path); - if (fd < 0) { - seaf_warning ("[obj backend] Failed to open tmp file %s: %s.\n", - tmp_path, strerror(errno)); - return -1; - } - - if (writen (fd, data, len) < 0) { - seaf_warning ("[obj backend] Failed to write obj %s: %s.\n", - tmp_path, strerror(errno)); - return -1; - } - + if (fd < 0) + goto out; + if (writen (fd, data, len) < 0) + goto out; if (need_sync && fsync_obj_contents (fd) < 0) - return -1; - - /* Close may return error, especially in NFS. */ - if (close (fd) < 0) { - seaf_warning ("[obj backend Failed close obj %s: %s.\n", - tmp_path, strerror(errno)); - return -1; - } - + goto out; + close_ret = close (fd); + fd = -1; + if (close_ret < 0) + goto out; if (need_sync) { if (rename_and_sync (tmp_path, path) < 0) - return -1; - } else { - if (g_rename (tmp_path, path) < 0) { - seaf_warning ("[obj backend] Failed to rename %s: %s.\n", - path, strerror(errno)); - return -1; - } + goto out; + } else if (g_rename (tmp_path, path) < 0) { + goto out; } - - return 0; + ret = 0; +out: + saved_errno = errno; + if (fd >= 0) + close (fd); + if (ret < 0) { + seaf_warning ("[obj backend] Failed to publish object: %s.\n", + strerror (saved_errno)); + g_unlink (tmp_path); + } + g_free (tmp_path); + errno = saved_errno; + return ret; } static int @@ -385,6 +386,10 @@ obj_backend_fs_foreach_obj (ObjBackend *bend, pos = path + dir_len; while ((dname1 = g_dir_read_name(dir1)) != NULL) { + /* Only real object shards belong in the published object namespace. */ + if (strlen (dname1) != 2 || !g_ascii_isxdigit (dname1[0]) || + !g_ascii_isxdigit (dname1[1])) + continue; snprintf (pos, sizeof(path) - dir_len, "/%s", dname1); dir2 = g_dir_open (path, 0, NULL); @@ -394,7 +399,12 @@ obj_backend_fs_foreach_obj (ObjBackend *bend, } while ((dname2 = g_dir_read_name(dir2)) != NULL) { + /* Exclude current and historical staging names from GC/fsck. */ + if (strlen (dname2) != 38) + continue; snprintf (obj_id, sizeof(obj_id), "%s%s", dname1, dname2); + if (!is_object_id_valid (obj_id)) + continue; if (!process (repo_id, version, obj_id, user_data)) { g_dir_close (dir2); goto out; @@ -464,46 +474,83 @@ obj_backend_fs_remove_store (ObjBackend *bend, const char *dname1, *dname2; char *path1, *path2; guint64 removed_count = 0; + GError *error = NULL; + int ret = 0; obj_dir = g_build_filename (priv->obj_dir, store_id, NULL); + if (!obj_dir) + return -1; - dir1 = g_dir_open (obj_dir, 0, NULL); + dir1 = g_dir_open (obj_dir, 0, &error); if (!dir1) { g_free (obj_dir); - return 0; + if (error->code == G_FILE_ERROR_NOENT) { + g_clear_error (&error); + return 0; + } + seaf_warning ("Failed to open obj dir: %s.\n", error->message); + g_clear_error (&error); + return -1; } + GList *dir_list = NULL; + GList *ptr1; while ((dname1 = g_dir_read_name(dir1)) != NULL) { - path1 = g_build_filename (obj_dir, dname1, NULL); + dir_list = g_list_prepend(dir_list, g_strdup(dname1)); + } + g_dir_close (dir1); + + for (ptr1 = dir_list; ptr1; ptr1 = ptr1->next) { + path1 = g_build_filename (obj_dir, (char *)(ptr1->data), NULL); dir2 = g_dir_open (path1, 0, NULL); if (!dir2) { seaf_warning ("Failed to open obj dir %s.\n", path1); - g_dir_close (dir1); g_free (path1); - g_free (obj_dir); - return -1; + ret = -1; + continue; } + GList *file_list = NULL; + GList *ptr2; while ((dname2 = g_dir_read_name(dir2)) != NULL) { - path2 = g_build_filename (path1, dname2, NULL); - g_unlink (path2); + /* Never unlink a concurrent writer's staging file during removal. */ + if (g_str_has_prefix (dname2, ".cf-tmp-")) + continue; + file_list = g_list_prepend(file_list, g_strdup(dname2)); + } + g_dir_close (dir2); + + for (ptr2 = file_list; ptr2; ptr2 = ptr2->next) { + path2 = g_build_filename (path1, (char *)(ptr2->data), NULL); + if (g_unlink (path2) < 0) { + seaf_warning ("Failed to delete file %s.\n", path2); + ret = -1; + } ++removed_count; if (progress_cb) progress_cb (store_id, removed_count, user_data); g_free (path2); } - g_dir_close (dir2); - g_rmdir (path1); + if (g_rmdir (path1) < 0) { + seaf_warning ("Failed to delete dir %s.\n", path1); + ret = -1; + } g_free (path1); + + string_list_free (file_list); } - g_dir_close (dir1); - g_rmdir (obj_dir); + if (g_rmdir (obj_dir) < 0) { + seaf_warning ("Failed to delete dir %s.\n", obj_dir); + ret = -1; + } g_free (obj_dir); - return 0; + string_list_free (dir_list); + + return ret; } ObjBackend * diff --git a/fileserver/objstore/backend_fs.go b/fileserver/objstore/backend_fs.go index f1f42798..b0ff875d 100644 --- a/fileserver/objstore/backend_fs.go +++ b/fileserver/objstore/backend_fs.go @@ -2,16 +2,17 @@ package objstore import ( + "errors" "io" "os" "path" + "syscall" ) type fsBackend struct { // Path of the object directory objDir string objType string - tmpDir string } func newFSBackend(seafileDataDir string, objType string) (*fsBackend, error) { @@ -20,15 +21,9 @@ func newFSBackend(seafileDataDir string, objType string) (*fsBackend, error) { if err != nil { return nil, err } - tmpDir := path.Join(seafileDataDir, "tmpfiles") - err = os.MkdirAll(tmpDir, os.ModePerm) - if err != nil { - return nil, err - } backend := new(fsBackend) backend.objDir = objDir backend.objType = objType - backend.tmpDir = tmpDir return backend, nil } @@ -48,7 +43,7 @@ func (b *fsBackend) read(repoID string, objID string, w io.Writer) error { return nil } -func (b *fsBackend) write(repoID string, objID string, r io.Reader, sync bool) error { +func (b *fsBackend) write(repoID string, objID string, r io.Reader, needSync bool) error { parentDir := path.Join(b.objDir, repoID, objID[:2]) p := path.Join(parentDir, objID[2:]) err := os.MkdirAll(parentDir, os.ModePerm) @@ -56,16 +51,15 @@ func (b *fsBackend) write(repoID string, objID string, r io.Reader, sync bool) e return err } - tmpDir := b.tmpDir - if b.objType != "blocks" { - tmpDir = parentDir - } - tFile, err := os.CreateTemp(tmpDir, objID+".*") + // Stage beside the final object: even bind mounts of the same device reject + // cross-mount rename. The reserved name is never a published object ID. + tFile, err := os.CreateTemp(parentDir, ".cf-tmp-"+objID+"-*") if err != nil { return err } success := false defer func() { + tFile.Close() if !success { os.Remove(tFile.Name()) } @@ -77,6 +71,13 @@ func (b *fsBackend) write(repoID string, objID string, r io.Reader, sync bool) e return err } + // Honor the existing durability request before publication, as the native + // metadata writer does; callers that do not request it retain their policy. + if needSync { + if err = tFile.Sync(); err != nil && !errors.Is(err, syscall.EINVAL) { + return err + } + } err = tFile.Close() if err != nil { return err @@ -88,6 +89,19 @@ func (b *fsBackend) write(repoID string, objID string, r io.Reader, sync bool) e } success = true + if needSync { + // A successful rename alone does not durably persist its directory entry. + dir, err := os.Open(parentDir) + if err != nil { + return err + } + err = dir.Sync() + closeErr := dir.Close() + if err != nil && !errors.Is(err, syscall.EINVAL) { + return err + } + return closeErr + } return nil } diff --git a/fileserver/objstore/cf_local_atomic_linux_test.go b/fileserver/objstore/cf_local_atomic_linux_test.go new file mode 100644 index 00000000..7e3c4efa --- /dev/null +++ b/fileserver/objstore/cf_local_atomic_linux_test.go @@ -0,0 +1,56 @@ +package objstore + +// Requires a disposable Linux mount namespace; never use live data paths. +import ( + "bytes" + "errors" + "os" + "path/filepath" + "syscall" + "testing" +) + +func TestLocalAtomicBindMount(t *testing.T) { + if os.Getenv("CF_TEST_BIND_MOUNTS") != "1" { + t.Skip("requires isolated mount-capable Linux container") + } + for _, scope := range []string{"backend", "repository", "shard"} { + t.Run(scope, func(t *testing.T) { + root := t.TempDir() + b, err := newFSBackend(root, "blocks") + if err != nil { + t.Fatal(err) + } + target := filepath.Join(root, "storage", "blocks") + if scope != "backend" { + target = filepath.Join(target, repoID) + } + if scope == "shard" { + target = filepath.Join(target, objID[:2]) + } + if err = os.MkdirAll(target, 0700); err != nil { + t.Fatal(err) + } + backing := t.TempDir() + if err = syscall.Mount(backing, target, "", syscall.MS_BIND, ""); err != nil { + t.Fatal(err) + } + defer syscall.Unmount(target, 0) + probe := filepath.Join(root, "old-temp-probe") + if err = os.WriteFile(probe, []byte("x"), 0600); err != nil { + t.Fatal(err) + } + if err = os.Rename(probe, filepath.Join(target, "old-temp-probe")); !errors.Is(err, syscall.EXDEV) { + t.Fatalf("fixture did not reproduce EXDEV: %v", err) + } + payload := []byte("same-directory staging survives bind mount") + if err = b.write(repoID, objID, bytes.NewReader(payload), true); err != nil { + t.Fatal(err) + } + var got bytes.Buffer + if err = b.read(repoID, objID, &got); err != nil || !bytes.Equal(got.Bytes(), payload) { + t.Fatal("mounted roundtrip failed") + } + }) + } +} diff --git a/fileserver/objstore/cf_local_atomic_test.go b/fileserver/objstore/cf_local_atomic_test.go new file mode 100644 index 00000000..2110cd40 --- /dev/null +++ b/fileserver/objstore/cf_local_atomic_test.go @@ -0,0 +1,116 @@ +package objstore + +// Test actual publication behavior and deliberately unusable legacy staging. +import ( + "bytes" + "errors" + "io" + "os" + "path/filepath" + "strings" + "sync" + "testing" +) + +type atomicProbeReader struct { + probe func() + reader io.Reader +} + +func (r *atomicProbeReader) Read(p []byte) (int, error) { + if r.probe != nil { + f := r.probe + r.probe = nil + f() + } + return r.reader.Read(p) +} + +type atomicFailReader struct{} + +func (atomicFailReader) Read(p []byte) (int, error) { + copy(p, "partial") + return 7, errors.New("injected interrupted input") +} +func TestLocalAtomicStagingAndFailures(t *testing.T) { + for _, kind := range []string{"blocks", "fs", "commits"} { + t.Run(kind, func(t *testing.T) { + root := t.TempDir() + legacy := filepath.Join(root, "tmpfiles") + if err := os.WriteFile(legacy, []byte("not a directory"), 0600); err != nil { + t.Fatal(err) + } + b, err := newFSBackend(root, kind) + if err != nil { + t.Fatal(err) + } + shard := filepath.Join(root, "storage", kind, repoID, objID[:2]) + final := filepath.Join(shard, objID[2:]) + payload := []byte("complete immutable object bytes") + r := &atomicProbeReader{reader: bytes.NewReader(payload), probe: func() { + entries, e := os.ReadDir(shard) + if e != nil { + t.Fatal(e) + } + if len(entries) != 1 || !strings.HasPrefix(entries[0].Name(), ".cf-tmp-") { + t.Fatalf("staging: %v", entries) + } + if _, e = os.Stat(final); !os.IsNotExist(e) { + t.Fatal("object visible before completion") + } + }} + if err = b.write(repoID, objID, r, true); err != nil { + t.Fatal(err) + } + if err = b.write(repoID, objID, atomicFailReader{}, true); err == nil { + t.Fatal("interrupted write succeeded") + } + got, e := os.ReadFile(final) + if e != nil || !bytes.Equal(got, payload) { + t.Fatal("failed write changed published bytes") + } + entries, e := os.ReadDir(shard) + if e != nil || len(entries) != 1 || entries[0].Name() != objID[2:] { + t.Fatalf("temp leak: %v %v", entries, e) + } + if got, e = os.ReadFile(legacy); e != nil || string(got) != "not a directory" { + t.Fatal("legacy path changed") + } + }) + } +} +func TestLocalAtomicConcurrentPublication(t *testing.T) { + b, err := newFSBackend(t.TempDir(), "blocks") + if err != nil { + t.Fatal(err) + } + payload := bytes.Repeat([]byte("same content-addressed object"), 4096) + var group sync.WaitGroup + for i := 0; i < 8; i++ { + group.Add(1) + go func() { + defer group.Done() + if e := b.write(repoID, objID, bytes.NewReader(payload), false); e != nil { + t.Error(e) + } + }() + } + group.Wait() + var got bytes.Buffer + if err = b.read(repoID, objID, &got); err != nil || !bytes.Equal(got.Bytes(), payload) { + t.Fatal("concurrent publication corrupted object") + } +} +func TestLocalAtomicRoutedStorageIgnoresLegacyTemp(t *testing.T) { + b, root := v2Fixture(t, "blocks") + legacy := filepath.Join(root, repoID, "tmpfiles") + if err := os.WriteFile(legacy, []byte("blocked legacy staging"), 0600); err != nil { + t.Fatal(err) + } + if err := b.write(repoID, objID, strings.NewReader("routed complete bytes"), true); err != nil { + t.Fatal(err) + } + if data, err := os.ReadFile(legacy); err != nil || string(data) != "blocked legacy staging" { + t.Fatal("routed staging touched legacy tmpfiles") + } +} diff --git a/fileserver/objstore/cf_v2_storage.go b/fileserver/objstore/cf_v2_storage.go index 1e9d0a6a..1d9deaa3 100644 --- a/fileserver/objstore/cf_v2_storage.go +++ b/fileserver/objstore/cf_v2_storage.go @@ -123,7 +123,7 @@ func (b *v2StorageBackend) resolve(repoID string) (*fsBackend, error) { // Native code retains its own fs/block identifiers, including virtual repo // origin IDs. Only the physical root changes. No per-request pool or cache. return &fsBackend{objDir: filepath.Join(keyDir, "storage", b.native.objType), - objType: b.native.objType, tmpDir: filepath.Join(keyDir, "tmpfiles")}, nil + objType: b.native.objType}, nil } func (b *v2StorageBackend) read(repoID, objID string, w io.Writer) error { @@ -139,11 +139,8 @@ func (b *v2StorageBackend) write(repoID, objID string, r io.Reader, sync bool) e if err != nil { return err } - if child != b.native { - if err = os.MkdirAll(child.tmpDir, 0700); err != nil { - return err - } - } + // The object backend owns same-directory staging for both native and + // routed libraries; never create a separate per-library tmpfiles tree. return child.write(repoID, objID, r, sync) } diff --git a/tests/local_storage/block.c b/tests/local_storage/block.c new file mode 100644 index 00000000..0c56ba06 --- /dev/null +++ b/tests/local_storage/block.c @@ -0,0 +1,26 @@ +/* Compile the real backend, not a duplicate storage implementation. */ +#include "../../common/block-backend-fs.c" +static const char *repo="11111111-1111-4111-8111-111111111111"; +static const char *oid="abcdef0123456789012345678901234567890123"; +static gboolean count(const char *r,int v,const char *id,void *p) {g_assert_cmpstr(id,==,oid);(*(int*)p)++;return TRUE;} +int main(int argc,char **argv) { + g_assert_cmpint(argc,==,2);char *legacy=g_build_filename(argv[1],"tmpfiles",NULL); + g_assert_true(g_file_set_contents(legacy,"poison",-1,NULL)); + BlockBackend *b=block_backend_fs_new(argv[1],legacy);g_assert_nonnull(b); + BHandle *h=b->open_block(b,repo,1,oid,BLOCK_WRITE);g_assert_nonnull(h); + char final[SEAF_PATH_MAX];g_assert_nonnull(get_block_path(b,oid,final,repo,1)); + char *parent=g_path_get_dirname(final),*actual=g_path_get_dirname(h->tmp_file); + g_assert_cmpstr(parent,==,actual);g_assert_false(g_file_test(final,G_FILE_TEST_EXISTS)); + g_assert_cmpint(b->write_block(b,h,"complete",8),==,8); + int n=0;g_assert_cmpint(b->foreach_block(b,repo,1,count,&n),==,0);g_assert_cmpint(n,==,0); + g_assert_cmpint(b->close_block(b,h),==,0);g_assert_cmpint(b->commit_block(b,h),==,0);b->block_handle_free(b,h); + char *orphan=g_build_filename(parent,".cf-tmp-crashed",NULL),*old=g_strconcat(final,".XXXXXX",NULL); + g_assert_true(g_file_set_contents(orphan,"partial",-1,NULL));g_assert_true(g_file_set_contents(old,"partial",-1,NULL)); + n=0;g_assert_cmpint(b->foreach_block(b,repo,1,count,&n),==,0);g_assert_cmpint(n,==,1); + h=b->open_block(b,repo,1,oid,BLOCK_WRITE);g_assert_nonnull(h);char *aborted=g_strdup(h->tmp_file); + b->close_block(b,h);b->block_handle_free(b,h);g_assert_false(g_file_test(aborted,G_FILE_TEST_EXISTS)); + char *data=NULL;gsize len;g_assert_true(g_file_get_contents(final,&data,&len,NULL));g_assert_cmpmem(data,len,"complete",8); + /* Store removal must report a retained staging file, not unlink it. */ + g_assert_cmpint(b->remove_store(b,repo,NULL,NULL),==,-1);g_assert_true(g_file_test(orphan,G_FILE_TEST_EXISTS)); + return 0; +} diff --git a/tests/local_storage/object.c b/tests/local_storage/object.c new file mode 100644 index 00000000..c4c1c001 --- /dev/null +++ b/tests/local_storage/object.c @@ -0,0 +1,21 @@ +/* Exercise cleanup and enumeration using the real metadata backend. */ +#include "../../common/obj-backend-fs.c" +extern int fail_write; +static const char *repo="11111111-1111-4111-8111-111111111111"; +static const char *oid="abcdef0123456789012345678901234567890123"; +static gboolean count(const char *r,int v,const char *id,void *p) {g_assert_cmpstr(id,==,oid);(*(int*)p)++;return TRUE;} +int main(int argc,char **argv) { + g_assert_cmpint(argc,==,2);ObjBackend *b=obj_backend_fs_new(argv[1],"fs");g_assert_nonnull(b); + g_assert_cmpint(b->write(b,repo,1,oid,"complete",8,TRUE),==,0); + char final[SEAF_PATH_MAX];id_to_path(b->priv,oid,final,repo,1); + fail_write=1;g_assert_cmpint(b->write(b,repo,1,oid,"partial",7,TRUE),==,-1);fail_write=0; + char *parent=g_path_get_dirname(final);GDir *dir=g_dir_open(parent,0,NULL);const char *name;int n=0; + while ((name=g_dir_read_name(dir))) {g_assert_false(g_str_has_prefix(name,".cf-tmp-"));n++;}g_dir_close(dir);g_assert_cmpint(n,==,1); + char *orphan=g_build_filename(parent,".cf-tmp-crashed",NULL),*old=g_strconcat(final,".XXXXXX",NULL); + g_assert_true(g_file_set_contents(orphan,"partial",-1,NULL));g_assert_true(g_file_set_contents(old,"partial",-1,NULL)); + n=0;g_assert_cmpint(b->foreach_obj(b,repo,1,count,&n),==,0);g_assert_cmpint(n,==,1); + char *data=NULL;gsize len;g_assert_true(g_file_get_contents(final,&data,&len,NULL));g_assert_cmpmem(data,len,"complete",8); + /* Store removal must leave an active or crashed writer visible for maintenance. */ + g_assert_cmpint(b->remove_store(b,repo,NULL,NULL),==,-1);g_assert_true(g_file_test(orphan,G_FILE_TEST_EXISTS)); + return 0; +} diff --git a/tests/local_storage/run.sh b/tests/local_storage/run.sh new file mode 100644 index 00000000..5e698b88 --- /dev/null +++ b/tests/local_storage/run.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# Compile actual native sources against isolated routing/I/O test seams. +set -euo pipefail +repo_root=$(cd "$(dirname "$0")/../.." && pwd) +fixture=$(mktemp -d) +trap 'rm -rf "$fixture"' EXIT +read -r -a glib_cflags <<< "$(pkg-config --cflags glib-2.0 gobject-2.0)" +read -r -a glib_libs <<< "$(pkg-config --libs glib-2.0 gobject-2.0)" +for name in block object; do + cc -D_GNU_SOURCE -I"$repo_root/common" -I"$repo_root/lib" "${glib_cflags[@]}" \ + "$repo_root/tests/local_storage/$name.c" "$repo_root/tests/local_storage/stubs.c" \ + "${glib_libs[@]}" -o "$fixture/$name" + mkdir "$fixture/$name-data" + "$fixture/$name" "$fixture/$name-data" +done +printf 'Native atomic staging, abort cleanup and enumeration passed\n' diff --git a/tests/local_storage/stubs.c b/tests/local_storage/stubs.c new file mode 100644 index 00000000..7e6648a8 --- /dev/null +++ b/tests/local_storage/stubs.c @@ -0,0 +1,20 @@ +/* Test-only routing and I/O seams. Backend functions come from real sources. */ +#include "common.h" +#include "utils.h" +#include "cf-v2-storage.h" +int fail_write; +ssize_t readn(int fd, void *p, size_t n) { return read(fd,p,n); } +ssize_t writen(int fd, const void *p, size_t n) { if (fail_write) { errno=EIO;return -1; } return write(fd,p,n); } +void string_list_free(GList *list) { g_list_free_full(list,g_free); } +gboolean is_object_id_valid(const char *id) { + if (!id || strlen(id)!=40) return FALSE; + for(int i=0;i<40;i++) if (!(g_ascii_isdigit(id[i]) || (id[i]>='a' && id[i]<='f'))) return FALSE; + return TRUE; +} +char *cf_v2_store_dir(const char *root,const char *repo) { return g_build_filename(root,repo,NULL); } +gboolean cf_v2_storage_enabled(void) { return FALSE; } +int cf_v2_storage_copy(const char *src,const char *dst) { return link(src,dst); } +int seaf_stat(const char *path, SeafStat *st) { return stat(path,st); } +int seaf_fstat(int fd, SeafStat *st) { return fstat(fd,st); } +#include "log.h" +void seafile_debug_impl(SeafileDebugFlags flag,const gchar *format,...) { } From 99135b2ee3d473723cc6fa0065832b74bb7f92ff Mon Sep 17 00:00:00 2001 From: servicememeai Date: Tue, 20 Oct 2026 13:12:39 +0900 Subject: [PATCH 2/7] fix(storage): preserve unknown files during whole-store removal --- common/block-backend-fs.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/common/block-backend-fs.c b/common/block-backend-fs.c index d056dac1..9b8dcfe1 100644 --- a/common/block-backend-fs.c +++ b/common/block-backend-fs.c @@ -413,6 +413,10 @@ block_backend_fs_remove_store (BlockBackend *bend, GList *dir_list = NULL; GList *ptr1; while ((dname1 = g_dir_read_name(dir1)) != NULL) { + /* Unknown shard directories remain untouched and make removal fail. */ + if (strlen (dname1) != 2 || !g_ascii_isxdigit (dname1[0]) || + !g_ascii_isxdigit (dname1[1])) + continue; dir_list = g_list_prepend(dir_list, g_strdup(dname1)); } g_dir_close (dir1); @@ -431,11 +435,17 @@ block_backend_fs_remove_store (BlockBackend *bend, GList *file_list = NULL; GList *ptr2; while ((dname2 = g_dir_read_name(dir2)) != NULL) { - /* Whole-store removal must not unlink an in-flight new-format - * staging file; a nonempty shard makes removal fail visibly. */ - if (g_str_has_prefix (dname2, ".cf-tmp-")) + /* Remove published blocks/objects only. Preserve active staging, + * historical partial names and unknown files for maintenance; + * the nonempty directory then causes a visible removal failure. */ + if (strlen (dname2) != 38) + continue; + char published_id[41]; + g_snprintf (published_id, sizeof(published_id), "%s%s", + (const char *)(ptr1->data), dname2); + if (!is_object_id_valid (published_id)) continue; - file_list = g_list_prepend(file_list, g_strdup(dname2)); + file_list = g_list_prepend (file_list, g_strdup (dname2)); } g_dir_close (dir2); From 6865d91a924d267cd6145e76c21b29f1fa8491dc Mon Sep 17 00:00:00 2001 From: servicememeai Date: Tue, 20 Oct 2026 13:12:43 +0900 Subject: [PATCH 3/7] fix(storage): preserve unknown files during whole-store removal --- common/obj-backend-fs.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/common/obj-backend-fs.c b/common/obj-backend-fs.c index 9383d50d..94a4dbd2 100644 --- a/common/obj-backend-fs.c +++ b/common/obj-backend-fs.c @@ -496,6 +496,10 @@ obj_backend_fs_remove_store (ObjBackend *bend, GList *dir_list = NULL; GList *ptr1; while ((dname1 = g_dir_read_name(dir1)) != NULL) { + /* Unknown shard directories remain untouched and make removal fail. */ + if (strlen (dname1) != 2 || !g_ascii_isxdigit (dname1[0]) || + !g_ascii_isxdigit (dname1[1])) + continue; dir_list = g_list_prepend(dir_list, g_strdup(dname1)); } g_dir_close (dir1); @@ -514,10 +518,17 @@ obj_backend_fs_remove_store (ObjBackend *bend, GList *file_list = NULL; GList *ptr2; while ((dname2 = g_dir_read_name(dir2)) != NULL) { - /* Never unlink a concurrent writer's staging file during removal. */ - if (g_str_has_prefix (dname2, ".cf-tmp-")) + /* Remove published blocks/objects only. Preserve active staging, + * historical partial names and unknown files for maintenance; + * the nonempty directory then causes a visible removal failure. */ + if (strlen (dname2) != 38) + continue; + char published_id[41]; + g_snprintf (published_id, sizeof(published_id), "%s%s", + (const char *)(ptr1->data), dname2); + if (!is_object_id_valid (published_id)) continue; - file_list = g_list_prepend(file_list, g_strdup(dname2)); + file_list = g_list_prepend (file_list, g_strdup (dname2)); } g_dir_close (dir2); From ec9d95e9352743ec99ff800b0ffe8a6ced474d15 Mon Sep 17 00:00:00 2001 From: servicememeai Date: Tue, 20 Oct 2026 13:13:40 +0900 Subject: [PATCH 4/7] test(storage): retain legacy staging and unexpected shard contents --- tests/local_storage/block.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/local_storage/block.c b/tests/local_storage/block.c index 0c56ba06..540ddee1 100644 --- a/tests/local_storage/block.c +++ b/tests/local_storage/block.c @@ -20,7 +20,14 @@ int main(int argc,char **argv) { h=b->open_block(b,repo,1,oid,BLOCK_WRITE);g_assert_nonnull(h);char *aborted=g_strdup(h->tmp_file); b->close_block(b,h);b->block_handle_free(b,h);g_assert_false(g_file_test(aborted,G_FILE_TEST_EXISTS)); char *data=NULL;gsize len;g_assert_true(g_file_get_contents(final,&data,&len,NULL));g_assert_cmpmem(data,len,"complete",8); - /* Store removal must report a retained staging file, not unlink it. */ - g_assert_cmpint(b->remove_store(b,repo,NULL,NULL),==,-1);g_assert_true(g_file_test(orphan,G_FILE_TEST_EXISTS)); + /* Unknown shard names and both old/new staging formats stay untouched. */ + char *root=g_path_get_dirname(parent),*unknown_dir=g_build_filename(root,"zz",NULL); + char *unknown=g_build_filename(unknown_dir,"do-not-delete",NULL); + g_assert_cmpint(g_mkdir_with_parents(unknown_dir,0700),==,0); + g_assert_true(g_file_set_contents(unknown,"foreign",-1,NULL)); + g_assert_cmpint(b->remove_store(b,repo,NULL,NULL),==,-1); + g_assert_true(g_file_test(orphan,G_FILE_TEST_EXISTS)); + g_assert_true(g_file_test(old,G_FILE_TEST_EXISTS)); + g_assert_true(g_file_test(unknown,G_FILE_TEST_EXISTS)); return 0; } From 7b469a0c51f67825a731b23a10d68f4a5426065d Mon Sep 17 00:00:00 2001 From: servicememeai Date: Tue, 20 Oct 2026 13:13:44 +0900 Subject: [PATCH 5/7] test(storage): retain legacy staging and unexpected shard contents --- tests/local_storage/object.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/local_storage/object.c b/tests/local_storage/object.c index c4c1c001..ee029a58 100644 --- a/tests/local_storage/object.c +++ b/tests/local_storage/object.c @@ -15,7 +15,14 @@ int main(int argc,char **argv) { g_assert_true(g_file_set_contents(orphan,"partial",-1,NULL));g_assert_true(g_file_set_contents(old,"partial",-1,NULL)); n=0;g_assert_cmpint(b->foreach_obj(b,repo,1,count,&n),==,0);g_assert_cmpint(n,==,1); char *data=NULL;gsize len;g_assert_true(g_file_get_contents(final,&data,&len,NULL));g_assert_cmpmem(data,len,"complete",8); - /* Store removal must leave an active or crashed writer visible for maintenance. */ - g_assert_cmpint(b->remove_store(b,repo,NULL,NULL),==,-1);g_assert_true(g_file_test(orphan,G_FILE_TEST_EXISTS)); + /* Unknown shard names and both old/new staging formats stay untouched. */ + char *root=g_path_get_dirname(parent),*unknown_dir=g_build_filename(root,"zz",NULL); + char *unknown=g_build_filename(unknown_dir,"do-not-delete",NULL); + g_assert_cmpint(g_mkdir_with_parents(unknown_dir,0700),==,0); + g_assert_true(g_file_set_contents(unknown,"foreign",-1,NULL)); + g_assert_cmpint(b->remove_store(b,repo,NULL,NULL),==,-1); + g_assert_true(g_file_test(orphan,G_FILE_TEST_EXISTS)); + g_assert_true(g_file_test(old,G_FILE_TEST_EXISTS)); + g_assert_true(g_file_test(unknown,G_FILE_TEST_EXISTS)); return 0; } From 5040c28f19f0b10045362121d1def43722b30a93 Mon Sep 17 00:00:00 2001 From: servicememeai Date: Tue, 20 Oct 2026 13:19:26 +0900 Subject: [PATCH 6/7] test(storage): supply isolated libevent socket type fixture --- tests/local_storage/include/evutil.h | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/local_storage/include/evutil.h diff --git a/tests/local_storage/include/evutil.h b/tests/local_storage/include/evutil.h new file mode 100644 index 00000000..144b3d5c --- /dev/null +++ b/tests/local_storage/include/evutil.h @@ -0,0 +1,9 @@ +/* Isolated C object-backend fixture only: avoid requiring libevent v1 headers + * on the minimal CI runner. Never install this directory in native binaries. + * The tested filesystem backends do not call the event/socket helpers. */ +#ifndef CF_LOCAL_STORAGE_TEST_EVUTIL_H +#define CF_LOCAL_STORAGE_TEST_EVUTIL_H + +typedef int evutil_socket_t; + +#endif From a037060dfb9b9dcc5814ab7dcf8ff517aeadfa55 Mon Sep 17 00:00:00 2001 From: servicememeai Date: Tue, 20 Oct 2026 13:19:41 +0900 Subject: [PATCH 7/7] test(storage): isolate C backend harness from unavailable libevent v1 headers --- tests/local_storage/run.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/local_storage/run.sh b/tests/local_storage/run.sh index 5e698b88..4ef0ed0b 100644 --- a/tests/local_storage/run.sh +++ b/tests/local_storage/run.sh @@ -7,7 +7,9 @@ trap 'rm -rf "$fixture"' EXIT read -r -a glib_cflags <<< "$(pkg-config --cflags glib-2.0 gobject-2.0)" read -r -a glib_libs <<< "$(pkg-config --libs glib-2.0 gobject-2.0)" for name in block object; do - cc -D_GNU_SOURCE -I"$repo_root/common" -I"$repo_root/lib" "${glib_cflags[@]}" \ + # The test-only include directory supplies only the socket typedef needed + # by lib/utils.h; production continues using the real libevent headers. + cc -D_GNU_SOURCE -I"$repo_root/tests/local_storage/include" -I"$repo_root/common" -I"$repo_root/lib" "${glib_cflags[@]}" \ "$repo_root/tests/local_storage/$name.c" "$repo_root/tests/local_storage/stubs.c" \ "${glib_libs[@]}" -o "$fixture/$name" mkdir "$fixture/$name-data"