Ask an armed region's page state under the freeze lock when a fault finds its range gone, so that a late write is not taken for a frozen page's and a read is not passed on past an arm.

The review found two windows in memory.c lazy_fault, where a fault holding frz_lock and lz_lock finds its address
neither armed nor in flight. It returned 0 there, and access_fault then asked for the page's state without a lock.
A write whose range a flight gave back while the write waited for frz_lock went on to thaw_fault, whose thaw released
every armed range of the 2 MiB region (lazy_drop_region_locked), each fetched under the lock. And after a release
that sets no hold (mem_lazy_skip's give-back of another buffer's range), the completion thread could arm the page
again between that 0 and access_fault's query, so a read saw an inaccessible page and went to the crash handler; that
one predates the ranges in flight.

Now a fault that finds a flight there waits for it as before. Otherwise it releases lz_lock, asks for the page's state
while it still holds frz_lock, under which no arm runs, and runs the access again when the page allows it: a read on
a readable page, a write on a read-write one. A write to a frozen page still goes on to thaw_fault, and an address
that is not ours to the crash handler.

The new late_write scenario of tests/lazy_fetch_check.py has a thread write a range whose release starts and ends
while the write waits for frz_lock: the write runs again and the region's other range stays armed. With the old
return of 0 it fails, the other range released. The read's window has no check: the arm would have to land between
two instructions. Not reproduced in the game before the change; lazy_fetch_check passed in all its scenarios.

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

docs/building.md

@@ -697,7 +697,8 @@ does not either when a range in flight in its region ends during that test. A fe
 of its range: freezes, thaws, arms, releases and copies of other ranges return at once meanwhile, also while a thaw
 fetches the armed ranges of its own regions, two ranges fetch at once, and more touching threads than the flight table
 holds all end. The touches of a range in flight wait for it and run again, a write among them without releasing the
-region's other ranges; thaws, mapping changes, releases of its buffer, arms and freezes see it. The same with
+region's other ranges, also when the flight ended while the write waited for the freeze lock; thaws, mapping changes,
+releases of its buffer, arms and freezes see it. The same with
 `wb_lazy_chunk = 1`, and with `wb_lazy_unlocked = 0` the fetch holds the freeze lock again, a thaw's too, and a
 release fetches one range at a time (the table of ranges in flight stays: a touch of one still waits for it alone).
 The "mem: lazy write-back" lines give the longest fetch, the longest wait of a fault for the freeze lock and the

runtime/memory.c

@@ -266,15 +266,17 @@ static volatile uint32_t lz_fetch_ms;      // plat_now_ms() | 1 at the end of th
 //   - an arm leaves its pages alone (mem_lazy_arm), a freeze its region (lzr), and mem_lazy_touches counts it.
 // Every segment that leaves the list with inaccessible pages becomes an entry in the same lz_lock hold, and an entry
 // ends only once its pages are read-write; lists and entries never overlap. So a fault that finds its address in
-// neither, holding frz_lock (no arm runs then), has an address that is not ours or no longer inaccessible. An entry
-// starts under frz_lock and lz_lock and ends under lz_lock alone, so a holder of frz_lock may wait for one: its fetch
-// takes the queue lock, and no holder of the queue lock takes frz_lock. Every store to lzr is made under lz_lock, from
-// the list, the entries and the region an arm is protecting (lz_arm_r). Fetches of different ranges run at once (the
-// back end has a slot for each, vk_memory.c lazy_fetch). NX2NAVI_WB_LAZY_UNLOCKED=0 brings back 0.10.1's locking: frz_lock
-// is kept from the moment a fault or a release takes a range out until its entry ends, a release takes one segment at a
-// time, mem_thaw releases under the lock, and vk_memory.c mirror_evict and hot_check release their victims under
-// pool_lock, so one fetch runs at a time. The entries stay with it: a touch of a range in flight waits for that entry
-// and runs again, the hold is set at the take-out, and mem_lazy_touches counts the range.
+// neither, holding frz_lock (no arm runs then), has an address that is not ours or no longer inaccessible: the page's
+// state, asked under that lock, decides whether the access runs again (lazy_fault), and an arm after it only has the
+// access fault here once more. An entry starts under frz_lock and lz_lock and ends under lz_lock alone, so a holder of
+// frz_lock may wait for one: its fetch takes the queue lock, and no holder of the queue lock takes frz_lock. Every
+// store to lzr is made under lz_lock, from the list, the entries and the region an arm is protecting (lz_arm_r).
+// Fetches of different ranges run at once (the back end has a slot for each, vk_memory.c lazy_fetch).
+// NX2NAVI_WB_LAZY_UNLOCKED=0 brings back 0.10.1's locking: frz_lock is kept from the moment a fault or a release takes
+// a range out until its entry ends, a release takes one segment at a time, mem_thaw releases under the lock, and
+// vk_memory.c mirror_evict and hot_check release their victims under pool_lock, so one fetch runs at a time. The
+// entries stay with it: a touch of a range in flight waits for that entry and runs again, the hold is set at the
+// take-out, and mem_lazy_touches counts the range.
 #define LZ_FLY 64          // more than the threads that touch armed pages at once; when full, a touch waits for room
 #define LZ_BATCH 16        // segments a release takes out together and fetches in one call (1 with the setting at 0)
 typedef struct { uint64_t lo, hi, h0, bytes; uint32_t seq, tid; uint8_t used; } LzFly;   // bytes: to fetch, 0 none
@@ -814,8 +816,9 @@ void mem_lazy_window(int open) {
     plat_mutex_unlock(&lz_lock);
 }
 
-// An access fault at a: 1 when a was armed or in flight and its pages are read-write again (the access runs again), 0
-// when a is none of the lazy write-back's.
+// An access fault at a: 1 when a was armed or in flight and its pages are read-write again, or when its page allows the
+// access by now (another thread gave it back): the access runs again. 0 when a is none of the lazy write-back's and its
+// page does not allow the access (a write to a frozen page goes on to thaw_fault).
 static int lazy_fault(uint64_t a, int write) {
     if (a >= RT_ADDRESS_LIMIT || !__atomic_load_n(&lzr[a >> FRZ_SHIFT], __ATOMIC_ACQUIRE)) return 0;
     uint32_t me = plat_thread_id();
@@ -834,11 +837,23 @@ static int lazy_fault(uint64_t a, int write) {
         plat_mutex_lock(&lz_lock);   // the window's test and the segment's leaving the list in one step
         int i = 0;
         while (i < lz_n && !(a >= lzs[i].lo && a < lzs[i].hi)) i++;
-        if (i == lz_n) {   // in flight since the look above; else not ours, or given back (no arm runs under frz_lock)
-            plat_mutex_unlock(&frz_lock);
-            int joined = lazy_fly_join_locked(a);
+        if (i == lz_n) {
+            if (lazy_fly_find_locked(a, a + 1, 0) >= 0) {   // in flight since the look above: wait for it alone
+                plat_mutex_unlock(&frz_lock);
+                lazy_fly_join_locked(a);
+                plat_mutex_unlock(&lz_lock);
+                return 1;
+            }
+            // Not ours, or given back since the fault: the page's state decides, asked under frz_lock alone (no system
+            // call under lz_lock), so that no arm makes the page inaccessible between the answer and the retry. A write
+            // to a page a flight gave back runs again here instead of going on to thaw_fault, whose thaw would release
+            // every armed range of the region; and a read is not passed to the crash handler because an arm (after a
+            // release that sets no hold, mem_lazy_skip's give-back) made the page inaccessible again once this returned.
             plat_mutex_unlock(&lz_lock);
-            return joined;
+            uint64_t pg = a & ~(PAGE - 1), end;
+            int st = plat_mem_state(pg, pg + PAGE, &end);
+            plat_mutex_unlock(&frz_lock);
+            return write ? st == PLAT_PROT_RW : st >= PLAT_PROT_READ;
         }
         uint32_t w = __atomic_load_n(&lz_win_tid, __ATOMIC_ACQUIRE);
         if (w && w != me && tries < 400) {   // up to 20 ms: after the hand-over the fetch has that part's writes

tests/lazy_fetch_check.c

@@ -268,6 +268,30 @@ int main(int argc, char **argv) {
         CHECK(armed(s2, K64), "the region's other range was released");
         CHECK(lz_other == 0, "%llu other releases", (unsigned long long)lz_other);
         CHECK(b.took < 250 && b.got == 0x77, "the write waited %.1f ms and read back %#x", b.took, b.got);
+    } else if (!strcmp(what, "late_write")) {
+        // B writes a range whose flight starts and ends while B waits for frz_lock in lazy_fault (this thread holds the
+        // lock and runs a release's flight under it, as a thaw does): B finds the page given back and writes again, and
+        // the region's other range stays armed (B went on to thaw_fault before, whose thaw released it)
+        uint64_t R = heap + 128 * M, s1 = R, s2 = R + M;
+        CHECK(arm_buf(s1, K64, 1) == K64 && arm_buf(s2, K64, 1) == K64, "arms");
+        stale(s1, K64, 1), stale(s2, K64, 1);
+        fetch_ns = 20000000ull;
+        plat_mutex_lock(&frz_lock);
+        Touch b;
+        void *tb = touch(&b, s1 + 0x20, 1);
+        plat_wait_ns(50000000);   // B waits for frz_lock in lazy_fault, past its look for a range in flight
+        int k[LZ_BATCH];
+        LzSeg sg[LZ_BATCH];
+        uint64_t rg[1][3] = {{s1, s1 + K64, 0}};
+        plat_mutex_lock(&lz_lock);
+        int nt = lazy_take_batch_locked((const uint64_t (*)[3])rg, 1, k, sg);
+        plat_mutex_unlock(&lz_lock);
+        if (nt > 0) lazy_fly_run(k, sg, nt);
+        plat_mutex_unlock(&frz_lock);
+        plat_thread_join(tb);
+        CHECK(nt == 1 && b.got == 0x77, "%d taken out, the write read back %#x", nt, b.got);
+        CHECK(armed(s2, K64) && lz_other == 0, "the region's other range was released (%llu other releases)",
+              (unsigned long long)lz_other);
     } else if (!strcmp(what, "concurrent")) {
         // two threads touch two ranges at once: their fetches run at the same time
         uint64_t s1 = heap + 40 * M, s2 = heap + 48 * M;

tests/lazy_fetch_check.py

@@ -27,6 +27,7 @@ SCENARIOS = {
     'no_stall': ('no_stall', {}),
     'join': ('join', {}),
     'loser': ('loser', {}),
+    'late_write': ('late_write', {}),
     'concurrent': ('concurrent', {}),
     'thaw': ('thaw', {}),
     'thaw_unlocked': ('thaw_unlocked', {}),