morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2445] [MERGED] fix(native/windows): share FILE_SHARE_DELETE so owned objects can be marked delete-pending #5234

New issue

Closed

opened 2026-10-31 01:05:01 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:01 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2445
Author: @maodns
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/windows-native-delete-share-mode

📝 Commits (3)

63214b6 fix(native/windows): share FILE_SHARE_DELETE so owned objects can be marked delete-pending
c81b4a8 fix(windows): anchor private runtime cleanup to directory handles
7f428a8 test(windows): assert cleanup sharing failure by code

📊 Changes

2 files changed (+100 additions, -31 deletions)

View changed files

📝 native/windows/src/filesystem.cc (+97 -27)
📝 scripts/verify-windows-native.mjs (+3 -4)

📄 Description

Problem

The Windows native authority self-test (binding.call("inspect", [])) fails closed
on stock Windows x64 under a non-elevated account, so every provider-backed
operation is rejected and the whole native authority is unusable:

REA Windows native controls are unavailable: Delete owned object by handle failed [...]
(Win32 5): Access is denied.

Reproduced deterministically on Windows 11 x64 (Node 22.22.2, non-admin):
inspect failed 15/15 — 100%, not intermittent.

Existing reports for this exact failure, both still open: #1701, #2196.
PR #1826 (merged) fixed the symptom — the cached failure plus leaked
rea-native-probe-* directories — but not the root cause; the addon still fails
here after that fix.

Root cause

Windows requires that every handle currently open on an object grants
FILE_SHARE_DELETE before that object can be marked delete-pending via
SetFileInformationByHandle(FileDispositionInfo). Two sites violate this:

createRelative() creates the runtime object with FILE_SHARE_READ only,
and the returned handle is retained for the runtime's whole lifetime
(Runtime::directory). closeRuntime() then has to mark that same object
delete-pending — but the retained handle never shared DELETE, so the
transition returns Win32 5.
closeRuntime()'s child handles have the same defect: they request
DELETE access while sharing only FILE_SHARE_READ, so the subsequent
dispose() call on those very handles is denied too.

Fix

Add FILE_SHARE_DELETE to both share masks:

createRelative() — FILE_SHARE_READ → FILE_SHARE_READ | FILE_SHARE_DELETE
closeRuntime() child open — same

Deliberately not adding FILE_SHARE_WRITE: the write-exclusion behaviour of
the retained leases is preserved (the in-code comments at the snapshot and
read-open sites document that exclusion as intentional), while the delete-pending
transition the cleanup path depends on becomes possible. Only the DELETE share bit
is required by the root cause.

--- a/native/windows/src/filesystem.cc
+++ b/native/windows/src/filesystem.cc
@@ -165,7 +165,8 @@ static Handle createRelative(HANDLE parent, const std::wstring& name, DWORD acce
   const auto status = NtCreateFile(&raw, access | SYNCHRONIZE, &attributes, &observation, nullptr,
-                                   FILE_ATTRIBUTE_NORMAL, FILE_SHARE_READ, FILE_CREATE,
+                                   FILE_ATTRIBUTE_NORMAL,
+                                   FILE_SHARE_READ | FILE_SHARE_DELETE, FILE_CREATE,
                                    FILE_SYNCHRONOUS_IO_NONALERT | FILE_OPEN_REPARSE_POINT |
                                    (directory ? FILE_DIRECTORY_FILE : FILE_NON_DIRECTORY_FILE), nullptr, 0);
@@ -369,7 +370,7 @@ void closeRuntime(Runtime& root) {
     Handle child(CreateFileW((L"\\\\?\\" + path).c_str(), DELETE | GENERIC_READ | FILE_WRITE_ATTRIBUTES,
-                            FILE_SHARE_READ, nullptr, OPEN_EXISTING,
+                            FILE_SHARE_READ | FILE_SHARE_DELETE, nullptr, OPEN_EXISTING,
                             FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS, nullptr));

Verification

All of the following were run through the real loader path
(WindowsNativeLoader + the manifest.json SHA-256 check), not by calling the
addon directly:

inspect passes; jobMemoryLimit / privateDacl / atomicJobAssignment
all report true.
10 consecutive inspect calls leave zero residual rea-native-probe-*
directories.
Full runtime lifecycle succeeds end-to-end: runtime_create → runtime_mkdir →
runtime_write → runtime_open → read (data read back correctly) → close →
runtime_close, with no residual directory.
runtime_snapshot (async) resolves with a correct SHA-256.
A real analysis run on a large application completes all stages.

Note on the build this was verified with

Verified with a locally cross-compiled addon (llvm-mingw, Node headers 22.19.0).
Because the local build tracks main while the installed npm package is 6.4.0,
inspectionSchema in dist/windows/WindowsNativeLoader.js needed
jobMemoryLimit: z.literal(true).optional() to accept the newer artifact. That is
a packaging/version-skew concern only and is not part of this patch — the
schema fix belongs to whichever branch adds that field.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2445 **Author:** [@maodns](https://github.com/maodns) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/windows-native-delete-share-mode` --- ### 📝 Commits (3) - [`63214b6`](https://github.com/morluto/rea/commit/63214b66de0591a2fb1ee8a80dc1e9bff8096d84) fix(native/windows): share FILE_SHARE_DELETE so owned objects can be marked delete-pending - [`c81b4a8`](https://github.com/morluto/rea/commit/c81b4a80a080a2d3e63e85b335b9d9f19200b5c3) fix(windows): anchor private runtime cleanup to directory handles - [`7f428a8`](https://github.com/morluto/rea/commit/7f428a8cefd8d401db2eadc7e030df2cd1a06eae) test(windows): assert cleanup sharing failure by code ### 📊 Changes **2 files changed** (+100 additions, -31 deletions) <details> <summary>View changed files</summary> 📝 `native/windows/src/filesystem.cc` (+97 -27) 📝 `scripts/verify-windows-native.mjs` (+3 -4) </details> ### 📄 Description ### Problem The Windows native authority self-test (`binding.call("inspect", [])`) fails closed on stock Windows x64 under a non-elevated account, so **every provider-backed operation is rejected** and the whole native authority is unusable: ``` REA Windows native controls are unavailable: Delete owned object by handle failed [...] (Win32 5): Access is denied. ``` Reproduced deterministically on Windows 11 x64 (Node 22.22.2, non-admin): `inspect` failed **15/15** — 100%, not intermittent. Existing reports for this exact failure, both still open: #1701, #2196. PR #1826 (merged) fixed the *symptom* — the cached failure plus leaked `rea-native-probe-*` directories — but not the root cause; the addon still fails here after that fix. ### Root cause Windows requires that **every** handle currently open on an object grants `FILE_SHARE_DELETE` before that object can be marked delete-pending via `SetFileInformationByHandle(FileDispositionInfo)`. Two sites violate this: 1. **`createRelative()`** creates the runtime object with `FILE_SHARE_READ` only, and the returned handle is **retained for the runtime's whole lifetime** (`Runtime::directory`). `closeRuntime()` then has to mark that same object delete-pending — but the retained handle never shared DELETE, so the transition returns Win32 5. 2. **`closeRuntime()`'s child handles** have the same defect: they request `DELETE` access while sharing only `FILE_SHARE_READ`, so the subsequent `dispose()` call *on those very handles* is denied too. ### Fix Add `FILE_SHARE_DELETE` to both share masks: - `createRelative()` — `FILE_SHARE_READ` → `FILE_SHARE_READ | FILE_SHARE_DELETE` - `closeRuntime()` child open — same Deliberately **not** adding `FILE_SHARE_WRITE`: the write-exclusion behaviour of the retained leases is preserved (the in-code comments at the snapshot and read-open sites document that exclusion as intentional), while the delete-pending transition the cleanup path depends on becomes possible. Only the DELETE share bit is required by the root cause. ```diff --- a/native/windows/src/filesystem.cc +++ b/native/windows/src/filesystem.cc @@ -165,7 +165,8 @@ static Handle createRelative(HANDLE parent, const std::wstring& name, DWORD acce const auto status = NtCreateFile(&raw, access | SYNCHRONIZE, &attributes, &observation, nullptr, - FILE_ATTRIBUTE_NORMAL, FILE_SHARE_READ, FILE_CREATE, + FILE_ATTRIBUTE_NORMAL, + FILE_SHARE_READ | FILE_SHARE_DELETE, FILE_CREATE, FILE_SYNCHRONOUS_IO_NONALERT | FILE_OPEN_REPARSE_POINT | (directory ? FILE_DIRECTORY_FILE : FILE_NON_DIRECTORY_FILE), nullptr, 0); @@ -369,7 +370,7 @@ void closeRuntime(Runtime& root) { Handle child(CreateFileW((L"\\\\?\\" + path).c_str(), DELETE | GENERIC_READ | FILE_WRITE_ATTRIBUTES, - FILE_SHARE_READ, nullptr, OPEN_EXISTING, + FILE_SHARE_READ | FILE_SHARE_DELETE, nullptr, OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS, nullptr)); ``` ### Verification All of the following were run **through the real loader path** (`WindowsNativeLoader` + the `manifest.json` SHA-256 check), not by calling the addon directly: - `inspect` passes; `jobMemoryLimit` / `privateDacl` / `atomicJobAssignment` all report `true`. - 10 consecutive `inspect` calls leave **zero** residual `rea-native-probe-*` directories. - Full runtime lifecycle succeeds end-to-end: `runtime_create` → `runtime_mkdir` → `runtime_write` → `runtime_open` → read (data read back correctly) → `close` → `runtime_close`, with **no residual directory**. - `runtime_snapshot` (async) resolves with a correct SHA-256. - A real analysis run on a large application completes all stages. ### Note on the build this was verified with Verified with a locally cross-compiled addon (llvm-mingw, Node headers 22.19.0). Because the local build tracks `main` while the installed npm package is 6.4.0, `inspectionSchema` in `dist/windows/WindowsNativeLoader.js` needed `jobMemoryLimit: z.literal(true).optional()` to accept the newer artifact. That is a packaging/version-skew concern only and is **not** part of this patch — the schema fix belongs to whichever branch adds that field. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:01 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5234

No description provided.

Rows
Columns