Make sure to take sqlite write locks upfront

Fun quirk of sqlite is that if you BEGIN DEFERRED and then do a read
statement and then another process concurrently does a write and then
the first process does a write statement on the same transaction, it
will immediately return SQLITE_BUSY because it's impossible to do a
write against a past version of the database. To fix this we need to use
BEGIN IMMEDIATE to take a write lock upfront on any transaction that
will need to write.

app/FileProvider/FileProviderEnumerator.m

@@ -62,7 +62,7 @@ - (void)enumerateItemsForObserver:(id<NSFileProviderEnumerationObserver>)observe
         if (strcmp(dirent->d_name, "..") == 0) {
             childIdent = _item.parentItemIdentifier;
         } else if (strcmp(dirent->d_name, ".") != 0) {
-            db_begin(&_item.mount->db);
+            db_begin_read(&_item.mount->db);
             inode_t inode = path_get_inode(&_item.mount->db, [path stringByAppendingFormat:@"/%@", [NSString stringWithUTF8String:dirent->d_name]].fileSystemRepresentation);
             db_commit(&_item.mount->db);
             if (inode == 0) {

app/FileProvider/FileProviderExtension.m

@@ -173,7 +173,7 @@ - (void)itemChangedAtURL:(NSURL *)url {
 // It's ok to use _mount in these because in each case the caller has already invoked itemForIdentifier:error: at least once
 - (BOOL)doCreateDirectoryAt:(NSString *)path inode:(ino_t *)inode error:(NSError **)error {
     NSURL *url = [[NSURL fileURLWithPath:[NSString stringWithUTF8String:_mount.source]] URLByAppendingPathComponent:path];
-    db_begin(&_mount.db);
+    db_begin_write(&_mount.db);
     if (![NSFileManager.defaultManager createDirectoryAtURL:url
                                 withIntermediateDirectories:NO
                                                  attributes:@{NSFilePosixPermissions: @0777}
@@ -198,7 +198,7 @@ - (BOOL)doCreateDirectoryAt:(NSString *)path inode:(ino_t *)inode error:(NSError
 
 - (BOOL)doCreateFileAt:(NSString *)path importFrom:(NSURL *)importURL inode:(ino_t *)inode error:(NSError **)error {
     NSURL *url = [[NSURL fileURLWithPath:[NSString stringWithUTF8String:_mount.source]] URLByAppendingPathComponent:path];
-    db_begin(&_mount.db);
+    db_begin_write(&_mount.db);
     if (![NSFileManager.defaultManager copyItemAtURL:importURL
                                                toURL:url
                                                error:error]) {
@@ -304,7 +304,7 @@ - (BOOL)doDelete:(NSString *)path itemIdentifier:(NSFileProviderItemIdentifier)i
         if (![self doDelete:[self pathFromURL:suburl] itemIdentifier:identifier error:error])
             return NO;
     }
-    db_begin(&_mount.db);
+    db_begin_write(&_mount.db);
     path_unlink(&_mount.db, path.fileSystemRepresentation);
     int err = unlinkat(_mount.root_fd, fix_path(path.fileSystemRepresentation), 0);
     if (err < 0)
@@ -332,7 +332,7 @@ - (void)deleteItemWithIdentifier:(NSFileProviderItemIdentifier)itemIdentifier co
 }
 
 - (BOOL)doRename:(NSString *)src to:(NSString *)dst itemIdentifier:(NSFileProviderItemIdentifier)identifier error:(NSError **)error {
-    db_begin(&_mount.db);
+    db_begin_write(&_mount.db);
     path_rename(&_mount.db, src.fileSystemRepresentation, dst.fileSystemRepresentation);
     int err = renameat(_mount.root_fd, fix_path(src.fileSystemRepresentation), _mount.root_fd, fix_path(dst.fileSystemRepresentation));
     if (err < 0) {
@@ -415,7 +415,7 @@ - (void)cleanupStorage {
             continue;
 
         // TODO: make this a function in fake-db.c
-        db_begin(&_mount.db);
+        db_begin_read(&_mount.db);
         sqlite3_bind_int64(_mount.db.stmt.inode_read_stat, 1, inode);
         BOOL exists = db_exec(&_mount.db, _mount.db.stmt.inode_read_stat);
         db_reset(&_mount.db, _mount.db.stmt.inode_read_stat);

app/FileProvider/FileProviderItem.m

@@ -43,7 +43,7 @@ - (int)openNewFDWithError:(NSError *__autoreleasing  _Nullable *)error {
     if (self.isRoot) {
         fd = open(_mount->source, O_DIRECTORY | O_RDONLY);
     } else {
-        db_begin(&_mount->db);
+        db_begin_read(&_mount->db);
         sqlite3_stmt *stmt = _mount->db.stmt.path_from_inode;
         sqlite3_bind_int64(_mount->db.stmt.path_from_inode, 1, _identifier.longLongValue);
         while (db_exec(&_mount->db, stmt)) {
@@ -87,7 +87,7 @@ - (NSURL *)URL {
 
 - (struct ish_stat)ishStat {
     struct ish_stat stat = {};
-    db_begin(&_mount->db);
+    db_begin_read(&_mount->db);
     inode_t inode = _identifier.longLongValue;
     if ([_identifier isEqualToString:NSFileProviderRootContainerItemIdentifier])
         inode = path_get_inode(&_mount->db, "");
@@ -115,7 +115,7 @@ - (NSFileProviderItemIdentifier)parentItemIdentifier {
     NSString *parentPath = self.path.stringByDeletingLastPathComponent;
     if ([parentPath isEqualToString:@"/"])
         return NSFileProviderRootContainerItemIdentifier;
-    db_begin(&_mount->db);
+    db_begin_read(&_mount->db);
     inode_t parentInode = path_get_inode(&_mount->db, parentPath.UTF8String);
     db_commit(&_mount->db);
     assert(parentInode != 0);

fs/fake-db.c

@@ -39,9 +39,13 @@ void db_exec_reset(struct fakefs_db *fs, sqlite3_stmt *stmt) {
     db_reset(fs, stmt);
 }
 
-void db_begin(struct fakefs_db *fs) {
+void db_begin_read(struct fakefs_db *fs) {
     sqlite3_mutex_enter(fs->lock);
-    db_exec_reset(fs, fs->stmt.begin);
+    db_exec_reset(fs, fs->stmt.begin_deferred);
+}
+void db_begin_write(struct fakefs_db *fs) {
+    sqlite3_mutex_enter(fs->lock);
+    db_exec_reset(fs, fs->stmt.begin_immediate);
 }
 void db_commit(struct fakefs_db *fs) {
     db_exec_reset(fs, fs->stmt.commit);
@@ -243,7 +247,8 @@ int fake_db_init(struct fakefs_db *fs, const char *db_path, int root_fd) {
     sqlite3_finalize(statement);
 
     fs->lock = sqlite3_mutex_alloc(SQLITE_MUTEX_FAST);
-    fs->stmt.begin = db_prepare(fs, "begin");
+    fs->stmt.begin_deferred = db_prepare(fs, "begin deferred");
+    fs->stmt.begin_immediate = db_prepare(fs, "begin immediate");
     fs->stmt.commit = db_prepare(fs, "commit");
     fs->stmt.rollback = db_prepare(fs, "rollback");
     fs->stmt.path_get_inode = db_prepare(fs, "select inode from paths where path = ?");
@@ -263,7 +268,8 @@ int fake_db_init(struct fakefs_db *fs, const char *db_path, int root_fd) {
 
 int fake_db_deinit(struct fakefs_db *fs) {
     if (fs->db) {
-        sqlite3_finalize(fs->stmt.begin);
+        sqlite3_finalize(fs->stmt.begin_deferred);
+        sqlite3_finalize(fs->stmt.begin_immediate);
         sqlite3_finalize(fs->stmt.commit);
         sqlite3_finalize(fs->stmt.rollback);
         sqlite3_finalize(fs->stmt.path_get_inode);

fs/fake-db.h

@@ -8,7 +8,8 @@
 struct fakefs_db {
     sqlite3 *db;
     struct {
-        sqlite3_stmt *begin;
+        sqlite3_stmt *begin_deferred;
+        sqlite3_stmt *begin_immediate;
         sqlite3_stmt *commit;
         sqlite3_stmt *rollback;
         sqlite3_stmt *path_get_inode;
@@ -29,7 +30,8 @@ struct fakefs_db {
 int fake_db_init(struct fakefs_db *fs, const char *db_path, int root_fd);
 int fake_db_deinit(struct fakefs_db *fs);
 
-void db_begin(struct fakefs_db *fs);
+void db_begin_read(struct fakefs_db *fs);
+void db_begin_write(struct fakefs_db *fs);
 void db_commit(struct fakefs_db *fs);
 void db_rollback(struct fakefs_db *fs);
 

fs/fake.c

@@ -26,7 +26,7 @@ static struct fd *fakefs_open(struct mount *mount, const char *path, int flags,
     struct fd *fd = realfs.open(mount, path, flags, 0666);
     if (IS_ERR(fd))
         return fd;
-    db_begin(fs);
+    db_begin_write(fs);
     fd->fake_inode = path_get_inode(fs, path);
     if (flags & O_CREAT_) {
         struct ish_stat ishstat;
@@ -53,7 +53,7 @@ static struct fd *fakefs_open(struct mount *mount, const char *path, int flags,
 // WARNING: giant hack, just for file providerws
 struct fd *fakefs_open_inode(struct mount *mount, ino_t inode) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_read(fs);
     sqlite3_stmt *stmt = fs->stmt.path_from_inode;
     sqlite3_bind_int64(stmt, 1, inode);
 step:
@@ -77,7 +77,7 @@ struct fd *fakefs_open_inode(struct mount *mount, ino_t inode) {
 
 static int fakefs_link(struct mount *mount, const char *src, const char *dst) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_write(fs);
     int err = realfs.link(mount, src, dst);
     if (err < 0) {
         db_rollback(fs);
@@ -90,7 +90,7 @@ static int fakefs_link(struct mount *mount, const char *src, const char *dst) {
 
 static int fakefs_unlink(struct mount *mount, const char *path) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_write(fs);
     int err = realfs.unlink(mount, path);
     if (err < 0) {
         db_rollback(fs);
@@ -104,7 +104,7 @@ static int fakefs_unlink(struct mount *mount, const char *path) {
 
 static int fakefs_rmdir(struct mount *mount, const char *path) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_write(fs);
     int err = realfs.rmdir(mount, path);
     if (err < 0) {
         db_rollback(fs);
@@ -118,7 +118,7 @@ static int fakefs_rmdir(struct mount *mount, const char *path) {
 
 static int fakefs_rename(struct mount *mount, const char *src, const char *dst) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_write(fs);
     path_rename(fs, src, dst);
     int err = realfs.rename(mount, src, dst);
     if (err < 0) {
@@ -131,7 +131,7 @@ static int fakefs_rename(struct mount *mount, const char *src, const char *dst)
 
 static int fakefs_symlink(struct mount *mount, const char *target, const char *link) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_write(fs);
     // create a file containing the target
     int fd = openat(mount->root_fd, fix_path(link), O_WRONLY | O_CREAT | O_EXCL, 0666);
     if (fd < 0) {
@@ -166,7 +166,7 @@ static int fakefs_mknod(struct mount *mount, const char *path, mode_t_ mode, dev
         real_mode |= S_IFREG;
     else
         real_mode |= mode & S_IFMT;
-    db_begin(fs);
+    db_begin_write(fs);
     int err = realfs.mknod(mount, path, real_mode, 0);
     if (err < 0) {
         db_rollback(fs);
@@ -186,7 +186,7 @@ static int fakefs_mknod(struct mount *mount, const char *path, mode_t_ mode, dev
 
 static int fakefs_stat(struct mount *mount, const char *path, struct statbuf *fake_stat) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_read(fs);
     struct ish_stat ishstat;
     ino_t inode;
     if (!path_read_stat(fs, path, &ishstat, &inode)) {
@@ -210,7 +210,7 @@ static int fakefs_fstat(struct fd *fd, struct statbuf *fake_stat) {
     int err = realfs.fstat(fd, fake_stat);
     if (err < 0)
         return err;
-    db_begin(fs);
+    db_begin_read(fs);
     struct ish_stat ishstat;
     if (!inode_read_stat_if_exist(fs, fd->fake_inode, &ishstat)) {
         db_rollback(fs);
@@ -245,7 +245,7 @@ static int fakefs_setattr(struct mount *mount, const char *path, struct attr att
     struct fakefs_db *fs = &mount->fakefs;
     if (attr.type == attr_size)
         return realfs.setattr(mount, path, attr);
-    db_begin(fs);
+    db_begin_read(fs);
     struct ish_stat ishstat;
     ino_t inode;
     if (!path_read_stat(fs, path, &ishstat, &inode)) {
@@ -262,7 +262,7 @@ static int fakefs_fsetattr(struct fd *fd, struct attr attr) {
     struct fakefs_db *fs = &fd->mount->fakefs;
     if (attr.type == attr_size)
         return realfs.fsetattr(fd, attr);
-    db_begin(fs);
+    db_begin_write(fs);
     struct ish_stat ishstat;
     inode_read_stat_or_die(fs, fd->fake_inode, &ishstat);
     fake_stat_setattr(&ishstat, attr);
@@ -273,7 +273,7 @@ static int fakefs_fsetattr(struct fd *fd, struct attr attr) {
 
 static int fakefs_mkdir(struct mount *mount, const char *path, mode_t_ mode) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_write(fs);
     int err = realfs.mkdir(mount, path, 0777);
     if (err < 0) {
         db_rollback(fs);
@@ -303,7 +303,7 @@ static ssize_t file_readlink(struct mount *mount, const char *path, char *buf, s
 
 static ssize_t fakefs_readlink(struct mount *mount, const char *path, char *buf, size_t bufsize) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_read(fs);
     struct ish_stat ishstat;
     if (!path_read_stat(fs, path, &ishstat, NULL)) {
         db_rollback(fs);
@@ -343,7 +343,7 @@ static int fakefs_readdir(struct fd *fd, struct dir_entry *entry) {
     }
 
     struct fakefs_db *fs = &fd->mount->fakefs;
-    db_begin(fs);
+    db_begin_read(fs);
     entry->inode = path_get_inode(fs, entry_path);
     db_commit(fs);
     // it's quite possible that due to some mishap there's no metadata for this file
@@ -389,7 +389,7 @@ static int fakefs_umount(struct mount *mount) {
 
 static void fakefs_inode_orphaned(struct mount *mount, ino_t inode) {
     struct fakefs_db *fs = &mount->fakefs;
-    db_begin(fs);
+    db_begin_write(fs);
     sqlite3_bind_int64(fs->stmt.try_cleanup_inode, 1, inode);
     db_exec_reset(fs, fs->stmt.try_cleanup_inode);
     db_commit(fs);

linux/fakefs.c

@@ -55,7 +55,7 @@ static struct dentry *fakefs_lookup(struct inode *ino, struct dentry *dentry, un
     char *path = dentry_name(dentry);
     if (IS_ERR(path))
         return ERR_PTR(PTR_ERR(path));
-    db_begin(&info->db);
+    db_begin_read(&info->db);
     inode_t child_ino = path_get_inode(&info->db, path);
     __putname(path);
     if (child_ino == 0)
@@ -124,7 +124,7 @@ static int __finish_make_node(struct user_namespace *mnt_userns, struct inode *d
         goto fail;
     }
 
-    db_begin(&info->db);
+    db_begin_write(&info->db);
     struct ish_stat ishstat = {
         .mode = mode,
         .uid = i_uid_read(child),
@@ -175,7 +175,7 @@ static int fakefs_rename(struct user_namespace *mnt_userns, struct inode *from_d
         return PTR_ERR(to_path);
     }
 
-    db_begin(&info->db);
+    db_begin_write(&info->db);
     path_rename(&info->db, from_path, to_path);
     __putname(from_path);
     __putname(to_path);
@@ -204,7 +204,7 @@ static int fakefs_link(struct dentry *from, struct inode *ino, struct dentry *to
         return PTR_ERR(to_path);
     }
 
-    db_begin(&info->db);
+    db_begin_write(&info->db);
     path_link(&info->db, from_path, to_path);
     __putname(from_path);
     __putname(to_path);
@@ -229,7 +229,7 @@ static int unlink_common(struct inode *dir, struct dentry *dentry, int is_dir) {
     if (IS_ERR(path))
         return PTR_ERR(path);
 
-    db_begin(&info->db);
+    db_begin_write(&info->db);
     path_unlink(&info->db, path);
     __putname(path);
 
@@ -289,7 +289,7 @@ static int fakefs_setattr(struct user_namespace *mnt_userns, struct dentry *dent
     // attributes of ishstat
     struct fakefs_super *info = inode->i_sb->s_fs_info;
     if (attr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) {
-        db_begin(&info->db);
+        db_begin_write(&info->db);
         struct ish_stat stat;
         inode_read_stat_or_die(&info->db, inode->i_ino, &stat);
         if (attr->ia_valid & ATTR_MODE)
@@ -412,7 +412,7 @@ static int fakefs_iterate(struct file *file, struct dir_context *ctx) {
         } else if (strcmp(ent.name, "..") == 0) {
             ent.ino = d_inode(file->f_path.dentry->d_parent)->i_ino;
         } else {
-            db_begin(&info->db);
+            db_begin_read(&info->db);
             if (dir_path_len + 1 + strlen(ent.name) + 1 > PATH_MAX)
                 continue; // a
             dir_path[dir_path_len] = '/';
@@ -651,7 +651,7 @@ static int fakefs_fill_super(struct super_block *sb, struct fs_context *fc) {
     struct inode *root = new_inode(sb);
     if (root == NULL)
         return -ENOMEM;
-    db_begin(&info->db);
+    db_begin_read(&info->db);
     root->i_ino = path_get_inode(&info->db, "");
     if (root->i_ino == 0) {
         printk("fakefs: could not find root inode\n");