Skip to content

Fix IDBFS autoPersist on mkdir #24799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/lib/libidbfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ addToLibrary({
// If the automatic IDBFS persistence option has been selected, then automatically persist
// all modifications to the filesystem as they occur.
if (mount?.opts?.autoPersist) {
mnt.idbPersistState = 0; // IndexedDB sync starts in idle state
mount.idbPersistState = 0; // IndexedDB sync starts in idle state
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a separate bug? What was the effect of this bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The effect wasn't observable to users because the code doesn't strictly check for equality with 0 and undefined is considered falsy:

if (!mount.idbPersistState) {

This fix is needed for the runOnceIDBFSIdle test helper.

var memfs_node_ops = mnt.node_ops;
mnt.node_ops = {...mnt.node_ops}; // Clone node_ops to inject write tracking
mnt.node_ops.mknod = (parent, name, mode, dev) => {
Expand Down Expand Up @@ -85,10 +85,12 @@ addToLibrary({
if (n.memfs_stream_ops.close) return n.memfs_stream_ops.close(stream);
};

// Persist the node we just created to IndexedDB
IDBFS.queuePersist(mnt.mount);

return node;
};
// Also kick off persisting the filesystem on other operations that modify the filesystem.
mnt.node_ops.mkdir = (...args) => (IDBFS.queuePersist(mnt.mount), memfs_node_ops.mkdir(...args));
mnt.node_ops.rmdir = (...args) => (IDBFS.queuePersist(mnt.mount), memfs_node_ops.rmdir(...args));
mnt.node_ops.symlink = (...args) => (IDBFS.queuePersist(mnt.mount), memfs_node_ops.symlink(...args));
mnt.node_ops.unlink = (...args) => (IDBFS.queuePersist(mnt.mount), memfs_node_ops.unlink(...args));
Expand Down
245 changes: 245 additions & 0 deletions test/fs/test_idbfs_autopersist.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,245 @@
/*
* Copyright 2025 The Emscripten Authors. All rights reserved.
* Emscripten is available under two separate licenses, the MIT license and the
* University of Illinois/NCSA Open Source License. Both these licenses can be
* found in the LICENSE file.
*/

#include <assert.h>
#include <stdbool.h>
#include <stdlib.h>
#include <stdio.h>
#include <emscripten.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/stat.h>
#include <errno.h>

EM_JS_DEPS(deps, "$callUserCallback");

enum {
TEST_CASE_OPEN,
TEST_CASE_CLOSE,
TEST_CASE_SYMLINK,
TEST_CASE_UNLINK,
TEST_CASE_RENAME,
TEST_CASE_MKDIR,
};

static void test_case_open(void) {
switch (TEST_PHASE) {
case 1: {
int fd = open("/working1/file", O_RDWR | O_CREAT | O_EXCL, 0777);
assert(fd != -1);
break;
}
case 2: {
struct stat st;
int res = lstat("/working1/file", &st);
assert(res == 0);
assert(st.st_size == 0);
break;
}
default:
assert(false);
}
}

static void test_case_close(void) {
switch (TEST_PHASE) {
case 1: {
int fd = open("/working1/file", O_RDWR | O_CREAT | O_EXCL, 0777);
assert(fd != -1);
break;
}
case 2: {
int fd = open("/working1/file", O_RDWR | O_CREAT, 0777);
assert(fd != -1);
ssize_t bytes_written = write(fd, "foo", 3);
assert(bytes_written == 3);
int res = close(fd);
assert(res == 0);
break;
}
case 3: {
struct stat st;
int res = lstat("/working1/file", &st);
assert(res == 0);
assert(st.st_size == 3);
break;
}
default:
assert(false);
}
}

static void test_case_symlink(void) {
switch (TEST_PHASE) {
case 1: {
int fd = open("/working1/file", O_RDWR | O_CREAT | O_EXCL, 0777);
assert(fd != -1);
break;
}
case 2: {
int res = symlink("/working1/file", "/working1/symlink");
assert(res == 0);
break;
}
case 3: {
struct stat st;
int res = lstat("/working1/symlink", &st);
assert(res == 0);
break;
}
default:
assert(false);
}
}

static void test_case_unlink(void) {
switch (TEST_PHASE) {
case 1: {
int fd = open("/working1/file", O_RDWR | O_CREAT | O_EXCL, 0777);
assert(fd != -1);
break;
}
case 2: {
int res = unlink("/working1/file");
assert(res == 0);
break;
}
case 3: {
struct stat st;
int res = lstat("/working1/file", &st);
assert(res == -1);
assert(errno == ENOENT);
break;
}
default:
assert(false);
}
}

static void test_case_rename(void) {
switch (TEST_PHASE) {
case 1: {
int fd = open("/working1/file", O_RDWR | O_CREAT | O_EXCL, 0777);
assert(fd != -1);
break;
}
case 2: {
int res = rename("/working1/file", "/working1/file_renamed");
assert(res == 0);
break;
}
case 3: {
struct stat st;
int res = lstat("/working1/file_renamed", &st);
assert(res == 0);
res = lstat("/working1/file", &st);
assert(res == -1);
assert(errno == ENOENT);
break;
}
default:
assert(false);
}
}

static void test_case_mkdir(void) {
switch (TEST_PHASE) {
case 1: {
int res = mkdir("/working1/dir", 0777);
assert(res == 0);
break;
}
case 2: {
struct stat st;
int res = lstat("/working1/dir", &st);
assert(res == 0);
break;
}
default:
assert(false);
}
}

EMSCRIPTEN_KEEPALIVE
void finish(void) {
emscripten_force_exit(0);
}

EMSCRIPTEN_KEEPALIVE
void test(void) {
switch (TEST_CASE) {
case TEST_CASE_OPEN: test_case_open(); break;
case TEST_CASE_CLOSE: test_case_close(); break;
case TEST_CASE_SYMLINK: test_case_symlink(); break;
case TEST_CASE_UNLINK: test_case_unlink(); break;
case TEST_CASE_RENAME: test_case_rename(); break;
case TEST_CASE_MKDIR: test_case_mkdir(); break;
default: assert(false);
}

EM_ASM({
// Wait until IDBFS has persisted before exiting
runOnceIDBFSIdle(() => {
callUserCallback(_finish);
});
});
}

int main(void) {
EM_ASM({
globalThis.runOnceIDBFSIdle = (callback) => {
const { mount } = FS.lookupPath('/working1').node;
assert('idbPersistState' in mount, 'mount object must have idbPersistState');
if (mount.idbPersistState !== 0) {
// IDBFS hasn't finished persisting. Check again after all pending tasks have executed
setTimeout(() => runOnceIDBFSIdle(callback), 0);
return;
}
callback();
};

FS.mkdir('/working1');
FS.mount(IDBFS, {
autoPersist: true
}, '/working1');
});

if (TEST_PHASE == 1) {
EM_ASM({
// The first phase of a test case must start from an empty filesystem.
// Erase persisted state by overwriting the contents of IndexedDB
// with our empty in-memory filesystem.
FS.syncfs(false, (err) => {
assert(!err);
callUserCallback(_test);
});
});
} else if (TEST_PHASE > 1) {
EM_ASM({
// All subsequent phases rely on the effects of phases before them.
// Load the persisted filesystem from IndexedDB into memory.
FS.syncfs(true, (err) => {
assert(!err);

// FS.syncfs() may run operations on the in-memory filesystem which
// might trigger IDBFS.queuePersist() calls. These queued calls will
// also persist modifications made by the test. We want to verify that
// each operation we test calls IDBFS.queuePersist() on its own, so
// the interference from FS.syncfs() is unwanted.
// Wait until the IDBFS mount has been persisted.
runOnceIDBFSIdle(() => {
callUserCallback(_test);
});
});
});
} else {
assert(false);
}

emscripten_exit_with_live_runtime();
return 0;
}
13 changes: 13 additions & 0 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,19 @@ def test_fs_idbfs_sync(self, args):
print('done first half')
self.btest_exit('fs/test_idbfs_sync.c', cflags=['-lidbfs.js', f'-DSECRET="{secret}"', '-sEXPORTED_FUNCTIONS=_main,_test,_finish', '-lidbfs.js'] + args)

@parameterized({
'open': ('TEST_CASE_OPEN', 2),
'close': ('TEST_CASE_CLOSE', 3),
'symlink': ('TEST_CASE_SYMLINK', 3),
'unlink': ('TEST_CASE_UNLINK', 3),
'rename': ('TEST_CASE_RENAME', 3),
'mkdir': ('TEST_CASE_MKDIR', 2),
})
def test_fs_idbfs_autopersist(self, test_case, phase_count):
self.cflags += ['-lidbfs.js', f'-DTEST_CASE={test_case}']
for phase in range(phase_count):
self.btest_exit('fs/test_idbfs_autopersist.c', cflags=[f'-DTEST_PHASE={phase + 1}'])

def test_fs_idbfs_fsync(self):
# sync from persisted state into memory before main()
self.set_setting('DEFAULT_LIBRARY_FUNCS_TO_INCLUDE', '$ccall')
Expand Down