Take the file lock at the end of a save file call only when a CloseFile is pending, not at the end of every call.

The review of the save file use count found that fs_host.c file_release took `lock` exclusive whenever users dropped
to 0, which is the end of every single read_file, write_file, file_size, set_file_size and flush_file, not only when
a CloseFile had marked the slot. The exclusive acquisition waits for every shared holder, such as the fsync loop of a
non-save Commit (commit, FlushFileBuffers under the shared lock), and while it waits it holds up new file_of callers:
a new wait on the guest threads that the change had said it did not add.

file_release now takes the lock only when it finds closing set after its decrement. The decrement and the load of
closing, and close_file's store of closing and load of users, are sequentially consistent pairs, so either the last
call out sees the mark or close_file sees users at 0 and closes the file itself; the check under the lock still keeps
the close to one. closing is an int32_t read and stored with atomics; a slot is emptied by the new file_clear, which
stores users and closing as atomics because the call that has just left the slot may read closing without the lock,
and open_file fills only h, mode and save.

Not reproduced in the game before the change. tests/fs_file_use_check.py gains a case: 1000 reads with no close take
the exclusive lock 0 times, and a read ends while another thread holds the lock shared. It failed on the code before
this change (1000 exclusive takes, the read waited) and passes after it, as do its close-during-a-read case and its
20000 closes and opens beside a reader (six runs, no foreign bytes, no double close). The compile check of fs_host.c
passes with no -Wall warnings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

runtime/fs_host.c

@@ -39,8 +39,8 @@ enum { M_FREE, M_SAVE, M_TEMP, M_CACHE, M_SAVE_RO };   // M_SAVE: the working co
 
 typedef struct { int kind; char name[32]; char root[1024]; } Mount;
 // h NULL = free; save: on a save-data mount; users: calls using h now (file_of .. file_release); closing: CloseFile
-// came while users > 0, and the last of them closes h
-typedef struct { PlatFile *h; int mode, save; int32_t users; int closing; } File;
+// came while users > 0, and the last of them closes h. users and closing are read without `lock` (file_release).
+typedef struct { PlatFile *h; int mode, save; int32_t users; int32_t closing; } File;
 typedef struct { char *name; int kind; uint64_t size; } Listed;
 typedef struct { Listed *e; int n, next, mode, used; } Dir;   // the directory's entries at open; used 0 = free
 
@@ -1151,14 +1151,16 @@ static uint32_t commit(const char *name) {
 // Commit first, save_touched), so a CloseFile on another thread could close it in the middle of a call. Windows hands
 // a closed HANDLE's value to the next file opened, so the call went on in another file: a save write could land in the
 // wrong one. Each call now counts itself in the slot from file_of to file_release. CloseFile closes at once when no
-// call is under way, and otherwise marks the slot; the last call out closes it. Nothing waits for another call, the
-// slot (and with it the guest's handle value) is not handed out again before the close, and a call that comes after
-// the CloseFile gets FS_R_INVALID_ARGUMENT, as a closed handle does.
+// call is under way, and otherwise marks the slot; the last call out closes it. Nothing waits for another call: a call
+// takes `lock` exclusive at its end only when it finds the slot marked, so the fsync loop a Commit runs under the
+// shared lock (commit) does not hold up the end of every read and write. The slot (and with it the guest's handle
+// value) is not handed out again before the close, and a call that comes after the CloseFile gets
+// FS_R_INVALID_ARGUMENT, as a closed handle does.
 static File *file_of(uint64_t h, PlatFile **fh, int *mode) {
     if (h < FILE_BASE || h >= FILE_BASE + NFILES) return NULL;
     plat_mutex_lock_shared(&lock);
     File *f = &files[h - FILE_BASE];
-    int ok = f->h && !f->closing;
+    int ok = f->h && !__atomic_load_n(&f->closing, __ATOMIC_RELAXED);   // stored only under the exclusive lock
     if (ok) {
         __atomic_add_fetch(&f->users, 1, __ATOMIC_ACQ_REL);   // shared holders count at once; close_file reads it exclusive
         if (fh) *fh = f->h;
@@ -1177,13 +1179,26 @@ static void file_closed(PlatFile *h, int save) {
     }
 }
 
-// The end of a call that file_of let in. The last call out of a slot CloseFile closed meanwhile closes the file.
+// Empties a slot, under the exclusive lock. users and closing are stored as atomics: the call that has just left the
+// slot may still read closing without the lock (file_release). open_file fills only h, mode and save.
+static void file_clear(File *f) {
+    f->h = NULL, f->mode = 0, f->save = 0;
+    __atomic_store_n(&f->users, 0, __ATOMIC_RELAXED);
+    __atomic_store_n(&f->closing, 0, __ATOMIC_RELAXED);
+}
+
+// The end of a call that file_of let in. The last call out of a slot CloseFile closed meanwhile closes the file. The
+// decrement and close_file's mark are a sequentially consistent pair: either this call sees closing, or close_file
+// sees users at 0 and closes the file itself. Only the first case takes the lock, and its check under the lock keeps
+// the close to one.
 static void file_release(File *f) {
-    if (__atomic_sub_fetch(&f->users, 1, __ATOMIC_ACQ_REL)) return;
+    if (__atomic_sub_fetch(&f->users, 1, __ATOMIC_SEQ_CST)) return;
+    if (!__atomic_load_n(&f->closing, __ATOMIC_SEQ_CST)) return;
     PlatFile *h = NULL;
     int save = 0;
     plat_mutex_lock(&lock);
-    if (f->h && f->closing && !__atomic_load_n(&f->users, __ATOMIC_ACQUIRE)) h = f->h, save = f->save, *f = (File){0};
+    if (f->h && __atomic_load_n(&f->closing, __ATOMIC_RELAXED) && !__atomic_load_n(&f->users, __ATOMIC_ACQUIRE))
+        h = f->h, save = f->save, file_clear(f);
     plat_mutex_unlock(&lock);
     if (h) file_closed(h, save);
 }
@@ -1215,8 +1230,8 @@ static uint32_t open_file(uint64_t *out, const char *path, int mode) {
     if (!f) return fs_result(err);
     plat_mutex_lock(&lock);
     for (int i = 0; i < NFILES; i++)
-        if (!files[i].h) {
-            files[i] = (File){.h = f, .mode = mode, .save = save_writer};
+        if (!files[i].h) {   // users and closing are 0 in a free slot (file_clear)
+            files[i].h = f, files[i].mode = mode, files[i].save = save_writer;
             *out = FILE_BASE + (uint64_t)i;
             plat_mutex_unlock(&lock);
             if (save_writer) {
@@ -1337,10 +1352,11 @@ static void close_file(uint64_t h) {
     PlatFile *close_now = NULL;
     int open = 0, save = 0;
     plat_mutex_lock(&lock);
-    if (f && f->h && !f->closing) {
+    if (f && f->h && !__atomic_load_n(&f->closing, __ATOMIC_RELAXED)) {
         open = 1;
-        if (__atomic_load_n(&f->users, __ATOMIC_ACQUIRE)) f->closing = 1;   // the last call under way closes it
-        else close_now = f->h, save = f->save, *f = (File){0};
+        // the mark first, then the count (file_release does the reverse): a call that is still under way closes it
+        __atomic_store_n(&f->closing, 1, __ATOMIC_SEQ_CST);
+        if (!__atomic_load_n(&f->users, __ATOMIC_SEQ_CST)) close_now = f->h, save = f->save, file_clear(f);
     }
     plat_mutex_unlock(&lock);
     if (!open) rt_fatal("fs: close of invalid file handle %#llx", (unsigned long long)h);

tests/fs_file_use_check.py

@@ -1,6 +1,8 @@
 """A save file's handle and a CloseFile on another thread (runtime/fs_host.c file_of, file_release, close_file): a
 call under way keeps its host file open, the close happens once when the last call is out, the guest's handle value
-is not handed out again before that, and a call after the close gets FS_R_INVALID_ARGUMENT. The host files here are
+is not handed out again before that, and a call after the close gets FS_R_INVALID_ARGUMENT. A call with no close
+pending takes no exclusive lock, so it ends while another thread holds the lock shared (a Commit's fsync loop). The
+host files here are
 stand-ins that hand a closed file's value to the next one opened, as Windows does with a HANDLE: a read that went on
 through a closed handle would read the other file. Builds fs_host.c's file functions into a test program. No game
 files.
@@ -46,7 +48,8 @@ HARNESS = r'''
 #include <string.h>
 typedef SRWLOCK PlatMutex;
 #define PLAT_MUTEX_INIT SRWLOCK_INIT
-static void plat_mutex_lock(PlatMutex *m) { AcquireSRWLockExclusive(m); }
+static volatile LONG exclusive_takes;   // exclusive acquisitions of fs_host.c's lock
+static void plat_mutex_lock(PlatMutex *m) { InterlockedIncrement(&exclusive_takes); AcquireSRWLockExclusive(m); }
 static void plat_mutex_unlock(PlatMutex *m) { ReleaseSRWLockExclusive(m); }
 static void plat_mutex_lock_shared(PlatMutex *m) { AcquireSRWLockShared(m); }
 static void plat_mutex_unlock_shared(PlatMutex *m) { ReleaseSRWLockShared(m); }
@@ -171,7 +174,25 @@ int main(void) {
     if (hc != ha) printf("FAIL: the closed slot was not free again (%#llx)\n", (unsigned long long)hc), fails++;
     close_file(hb), close_file(hc);
 
-    // 2. A reader loops over a file "a" while another thread closes it, opens "b" (which may take a closed handle's
+    // 2. Calls with no close pending take no exclusive lock: 1000 reads add none, and a read ends while another thread
+    //    holds the lock shared (fs_host.c commit's fsync loop runs so).
+    open_file(&ha, "a", FS_OPEN_READ);
+    LONG before = exclusive_takes;
+    int bad_reads = 0;
+    for (int i = 0; i < 1000; i++) bad_reads += read_file(&n, ha, 0, b, 4) != 0 || b[0] != 'a';
+    LONG taken = exclusive_takes - before;
+    AcquireSRWLockShared(&lock);
+    h_read = ha, got = 0;
+    t = CreateThread(NULL, 0, reader, NULL, 0, NULL);
+    int ended = WaitForSingleObject(t, 2000) == WAIT_OBJECT_0;
+    ReleaseSRWLockShared(&lock);
+    WaitForSingleObject(t, INFINITE);
+    printf("reads with no close pending: 1000 reads took the exclusive lock %ld time(s)%s, a read beside a shared holder "
+           "%s\n", (long)taken, taken ? " (FAIL)" : "", ended ? "ended" : "WAITED FOR IT (FAIL)");
+    if (taken || bad_reads || !ended || got != 'a') fails++;
+    close_file(ha);
+
+    // 3. A reader loops over a file "a" while another thread closes it, opens "b" (which may take a closed handle's
     //    value) and opens "a" again: no read returns b's bytes.
     open_file(&ha, "a", FS_OPEN_READ);
     current = ha;
@@ -205,7 +226,7 @@ def main():
         return 0
     file_t = re.search(r'typedef struct \{ PlatFile \*h;[^\n]*\} File;', source).group(0)
     parts = [HARNESS, file_t, 'static File files[NFILES];', 'static PlatMutex lock = PLAT_MUTEX_INIT;']
-    for sig in ('static File *file_of(', 'static void file_closed(', 'static void file_release(',
+    for sig in ('static File *file_of(', 'static void file_closed(', 'static void file_clear(', 'static void file_release(',
                 'static int64_t host_read_at(', 'static uint32_t open_file(', 'static uint32_t read_file(',
                 'static void close_file('):
         parts.append(function(sig))