Skip to content

Commit 0d7f63b

Browse files
committed
Fix IDBFS not auto-persisting on directory creation
FS.mkdir does not use mkdir under the hood but rather mknod. The patched version of mknod installed by IDBFS only invokes IDBFS.queuePersist() when the node's stream is closed, which never happens for mkdir as the handle is immediately discarded and no stream is ever created. Furthermore, the bare fact of file/directory creation should be persisted even if no writes are made to the node. The test doesn't catch this because reconcilation happens in a JavaScript task, and no tasks or microtasks can execute while code is executing. IDBFS.syncfs only has a chance to run after the entirety of the test function finishes executing, at which point all of file/directory operations have been executed and persisted in memory. As long as one of those operations calls IDBFS.queuePersist, it ensures that the whole memory filesystem will get persisted to IDB and therefore shadows missing calls to IDBFS.queuePersist in other operations. Remove the mkdir node op patching (which has never been a real node op) and make mknod queue a persist operation as soon as the node is created without waiting for the caller to interact with a stream. To fix the test, introduce an arbitrary split point between which IDBFS.syncfs has a chance to execute. However, this will only catch issues with mkdir. A real solution would be to wait after every syscall, but that will make the code unwieldy without something like JSPI.
1 parent be3fe6b commit 0d7f63b

File tree

3 files changed

+36
-3
lines changed

3 files changed

+36
-3
lines changed

src/lib/libidbfs.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,12 @@ addToLibrary({
8585
if (n.memfs_stream_ops.close) return n.memfs_stream_ops.close(stream);
8686
};
8787

88+
// Persist the node we just created to IndexedDB
89+
IDBFS.queuePersist(mnt.mount);
90+
8891
return node;
8992
};
9093
// Also kick off persisting the filesystem on other operations that modify the filesystem.
91-
mnt.node_ops.mkdir = (...args) => (IDBFS.queuePersist(mnt.mount), memfs_node_ops.mkdir(...args));
9294
mnt.node_ops.rmdir = (...args) => (IDBFS.queuePersist(mnt.mount), memfs_node_ops.rmdir(...args));
9395
mnt.node_ops.symlink = (...args) => (IDBFS.queuePersist(mnt.mount), memfs_node_ops.symlink(...args));
9496
mnt.node_ops.unlink = (...args) => (IDBFS.queuePersist(mnt.mount), memfs_node_ops.unlink(...args));

test/fs/test_idbfs_sync.c

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,32 @@ void test() {
6767
result = -11000 - errno;
6868
}
6969

70-
// a directory
70+
// a directory that we will later recreate from scratch before loading from IDB
7171
if ((stat("/working1/dir", &st) != -1) || (errno != ENOENT))
7272
result = -12000 - errno;
7373
else if (mkdir("/working1/dir", 0777) != 0)
7474
result = -13000 - errno;
7575

76+
EM_ASM(
77+
setTimeout(() => {
78+
// Run the remainder of the test after all already scheduled JavaScript
79+
// tasks have executed (such as the one scheduled by IDBFS.queuePersist).
80+
// This is to ensure that mkdir also triggers a call to queuePersist and
81+
// to isolate mkdir from queuePersist invocations made by other syscalls.
82+
ccall('test2', 'v');
83+
}, 0);
84+
);
85+
}
86+
87+
void test2() {
88+
struct stat st;
89+
90+
// a directory using which we will check if auto-persistence is working
91+
if ((stat("/working1/dir2", &st) != -1) || (errno != ENOENT))
92+
result = -30000 - errno;
93+
else if (mkdir("/working1/dir2", 0777) != 0)
94+
result = -31000 - errno;
95+
7696
#else
7797

7898
// does the empty file exist?
@@ -131,6 +151,16 @@ void test() {
131151
result = -29000 - errno;
132152
}
133153

154+
// does the other directory exist?
155+
if (stat("/working1/dir2", &st) != 0) {
156+
result = -32000 - errno;
157+
} else {
158+
if (!S_ISDIR(st.st_mode))
159+
result = -33000;
160+
if (rmdir("/working1/dir2") != 0)
161+
result = -34000 - errno;
162+
}
163+
134164
#endif
135165

136166
// If the test failed, then delete test files from IndexedDB so that the test
@@ -141,6 +171,7 @@ void test() {
141171
unlink("/working1/waka.txt");
142172
unlink("/working1/moar.txt");
143173
rmdir("/working1/dir");
174+
rmdir("/working1/dir2");
144175
EM_ASM(FS.syncfs(function(){})); // And persist deleted changes
145176
}
146177

test/test_browser.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1348,7 +1348,7 @@ def test_fflush(self):
13481348
def test_fs_idbfs_sync(self, args):
13491349
self.set_setting('DEFAULT_LIBRARY_FUNCS_TO_INCLUDE', '$ccall')
13501350
secret = str(time.time())
1351-
self.btest('fs/test_idbfs_sync.c', '1', cflags=['-lidbfs.js', f'-DSECRET="{secret}"', '-sEXPORTED_FUNCTIONS=_main,_test,_report_result', '-lidbfs.js'] + args + ['-DFIRST'])
1351+
self.btest('fs/test_idbfs_sync.c', '1', cflags=['-lidbfs.js', f'-DSECRET="{secret}"', '-sEXPORTED_FUNCTIONS=_main,_test,_test2,_report_result', '-lidbfs.js'] + args + ['-DFIRST'])
13521352
self.btest('fs/test_idbfs_sync.c', '1', cflags=['-lidbfs.js', f'-DSECRET="{secret}"', '-sEXPORTED_FUNCTIONS=_main,_test,_report_result', '-lidbfs.js'] + args)
13531353

13541354
def test_fs_idbfs_fsync(self):

0 commit comments

Comments
 (0)