morluto/rea

Fork 0

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

[PR #2202] [MERGED] fix(windows): preserve caller affinity only within one processor group #5080

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:03:19 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/windows-affinity-processor-groups

📝 Commits (3)

aca0503 fix(windows): preserve caller affinity only within one processor group
699a640 fix(windows): refuse to widen a pinned caller's affinity across groups
e1a2f2c docs(windows): record what owned-process affinity does not preserve

📊 Changes

5 files changed (+123 additions, -7 deletions)

View changed files

📝 docs/windows-ghidra-p0.md (+6 -2)
📝 native/windows/README.md (+16 -4)
📝 native/windows/src/process.cc (+29 -1)
📝 scripts/verify-windows-native.mjs (+36 -0)
📝 tests/fixtures/windows/processBoundary.cc (+36 -0)

📄 Description

Fixes #2181.

Cause

spawnProcess read the caller's affinity mask and applied it to the suspended
child unconditionally:

require(GetProcessAffinityMask(GetCurrentProcess(), &affinity, &systemAffinity), ...);
if (affinity != 0)
  require(SetProcessAffinityMask(result->process.get(), affinity), ...);

A mask names processors relative to one processor group, and Windows assigns
each new process to a group of its own choosing. On a host whose logical
processors span more than one group, the child is often placed in a group whose
active processors are not a superset of the caller's mask, so
SetProcessAffinityMask rejects it with ERROR_INVALID_PARAMETER (87). The
require made that fatal, so every owned provider spawn — the JavaScript
analysis worker and the other owned helpers — failed randomly depending on
which group Windows picked.

Change

Affinity is a scheduling nicety, not an authority, ownership or isolation
control, so it is now applied only when the caller and the suspended child both
occupy the same single processor group. A child placed in another group keeps
that group's default affinity and the spawn proceeds.

GetProcessGroupAffinity establishes membership for both processes. A process
whose threads span groups, or whose membership cannot be read, has no single
group to compare and is treated as unknown rather than as aligned, so no mask is
applied. Within a shared group the mask failure stays fatal, because there it is
not expected.

Unchanged: Job Object membership and kill-on-owner-close verification, breakaway
denial, child default-owner normalization, priority preservation, and the
suspended-create-then-resume sequence.

Single-group hosts — every host with 64 or fewer logical processors, and any
larger host whose caller and child land in the same group — behave exactly as
before: one group is reported for both processes and the same mask is applied.

Note for the >64-processor case on current Windows: since Windows 11 and Server
2022 a process's affinity spans all groups by default, so GetProcessAffinityMask
reports 0 and the existing affinity != 0 guard already skipped the call. The
reported host is Windows 10 19045, where the legacy per-group assignment applies
and the failure is reachable.

Tests and docs

tests/fixtures/windows/processBoundary.cc gains an affinity observation
reporting the process's own mask, whether it occupies a single processor group,
and which group.
scripts/verify-windows-native.mjs asserts that an owned spawn of that
fixture succeeds, and that the child's mask equals the caller's whenever both
report the same single group. On a multi-group host the exit-code assertion is
the regression test for this issue; the mask comparison is correctly skipped
because no mask is expected to carry across groups.
native/windows/README.md and docs/windows-ghidra-p0.md both stated that
caller CPU affinity is preserved without qualification. They now state the
processor-group condition and what a child in another group gets instead.

Validation

Windows 11 Pro 26100 x64, Node 24.16.0:

npm run docs:check — 6/6 tasks pass, including check-doc-facts.mjs.
npx oxfmt --check scripts/verify-windows-native.mjs — clean.
npm run lint — clean, module boundaries checked across 1,568 source files.
The max-lines warning on scripts/verify-windows-native.mjs predates this
change.
npm run build:cached — passes.
Windows x64 native artifact cross-built with the documented lane
(node scripts/build-windows-native.mjs on Linux with MinGW-w64 g++ 12 in a
container, since this host has no C++ toolchain). process.cc compiles
without new warnings. npm run verify:windows-native:artifact reports
ok: true for ABI 1 / Node-API 8, SHA-256
a036a9e4…5606e4, 842,929 bytes.
node scripts/verify-windows-native.mjs . <process-boundary.exe> on this real
Windows host against that artifact and an independently cross-built boundary
fixture: ok: true, all 20 filesystem controls and all 12 process
observations pass, including the new
callerAffinityPreservedWithinProcessorGroup. The caller observation was
{"affinityMask":4095,"singleProcessorGroup":true,"processorGroup":0} and the
child reported the same, so the mask-equality assertion ran rather than being
skipped.

Explicitly not verified: the failing multi-group path itself. This host has 12
logical processors, which is one processor group, so the rejected-mask branch
cannot occur here and only the unchanged single-group path is exercised. The
reasoning for the multi-group branch comes from the Win32 contract for
processor groups
and GetProcessGroupAffinity, plus the experimental analysis in #2181.
Confirmation on a host whose processors span more than one group — @hzh666kevin-hue's
88-processor, 2-group machine would do it — is still wanted, and the new
assertion is written to cover exactly that case.

🤖 Generated with Claude Code

_{🔄 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/2202 **Author:** [@Rinkia](https://github.com/Rinkia) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/windows-affinity-processor-groups` --- ### 📝 Commits (3) - [`aca0503`](https://github.com/morluto/rea/commit/aca0503d518d6f73636dd1901cd55aa9d94c1b3f) fix(windows): preserve caller affinity only within one processor group - [`699a640`](https://github.com/morluto/rea/commit/699a64034c0f99c24e85d48ba97e72fcb416396f) fix(windows): refuse to widen a pinned caller's affinity across groups - [`e1a2f2c`](https://github.com/morluto/rea/commit/e1a2f2caedb93025400e54193bee57f81d47c8b0) docs(windows): record what owned-process affinity does not preserve ### 📊 Changes **5 files changed** (+123 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `docs/windows-ghidra-p0.md` (+6 -2) 📝 `native/windows/README.md` (+16 -4) 📝 `native/windows/src/process.cc` (+29 -1) 📝 `scripts/verify-windows-native.mjs` (+36 -0) 📝 `tests/fixtures/windows/processBoundary.cc` (+36 -0) </details> ### 📄 Description Fixes #2181. ## Cause `spawnProcess` read the caller's affinity mask and applied it to the suspended child unconditionally: ```cpp require(GetProcessAffinityMask(GetCurrentProcess(), &affinity, &systemAffinity), ...); if (affinity != 0) require(SetProcessAffinityMask(result->process.get(), affinity), ...); ``` A mask names processors relative to one processor group, and Windows assigns each new process to a group of its own choosing. On a host whose logical processors span more than one group, the child is often placed in a group whose active processors are not a superset of the caller's mask, so `SetProcessAffinityMask` rejects it with `ERROR_INVALID_PARAMETER` (87). The `require` made that fatal, so every owned provider spawn — the JavaScript analysis worker and the other owned helpers — failed randomly depending on which group Windows picked. ## Change Affinity is a scheduling nicety, not an authority, ownership or isolation control, so it is now applied only when the caller and the suspended child both occupy the same single processor group. A child placed in another group keeps that group's default affinity and the spawn proceeds. `GetProcessGroupAffinity` establishes membership for both processes. A process whose threads span groups, or whose membership cannot be read, has no single group to compare and is treated as unknown rather than as aligned, so no mask is applied. Within a shared group the mask failure stays fatal, because there it is not expected. Unchanged: Job Object membership and kill-on-owner-close verification, breakaway denial, child default-owner normalization, priority preservation, and the suspended-create-then-resume sequence. Single-group hosts — every host with 64 or fewer logical processors, and any larger host whose caller and child land in the same group — behave exactly as before: one group is reported for both processes and the same mask is applied. Note for the >64-processor case on current Windows: since Windows 11 and Server 2022 a process's affinity spans all groups by default, so `GetProcessAffinityMask` reports 0 and the existing `affinity != 0` guard already skipped the call. The reported host is Windows 10 19045, where the legacy per-group assignment applies and the failure is reachable. ## Tests and docs - `tests/fixtures/windows/processBoundary.cc` gains an `affinity` observation reporting the process's own mask, whether it occupies a single processor group, and which group. - `scripts/verify-windows-native.mjs` asserts that an owned spawn of that fixture succeeds, and that the child's mask equals the caller's whenever both report the same single group. On a multi-group host the exit-code assertion is the regression test for this issue; the mask comparison is correctly skipped because no mask is expected to carry across groups. - `native/windows/README.md` and `docs/windows-ghidra-p0.md` both stated that caller CPU affinity is preserved without qualification. They now state the processor-group condition and what a child in another group gets instead. ## Validation Windows 11 Pro 26100 x64, Node 24.16.0: - `npm run docs:check` — 6/6 tasks pass, including `check-doc-facts.mjs`. - `npx oxfmt --check scripts/verify-windows-native.mjs` — clean. - `npm run lint` — clean, module boundaries checked across 1,568 source files. The `max-lines` warning on `scripts/verify-windows-native.mjs` predates this change. - `npm run build:cached` — passes. - Windows x64 native artifact cross-built with the documented lane (`node scripts/build-windows-native.mjs` on Linux with MinGW-w64 g++ 12 in a container, since this host has no C++ toolchain). `process.cc` compiles without new warnings. `npm run verify:windows-native:artifact` reports `ok: true` for ABI 1 / Node-API 8, SHA-256 `a036a9e4…5606e4`, 842,929 bytes. - `node scripts/verify-windows-native.mjs . <process-boundary.exe>` on this real Windows host against that artifact and an independently cross-built boundary fixture: `ok: true`, all 20 filesystem controls and all 12 process observations pass, including the new `callerAffinityPreservedWithinProcessorGroup`. The caller observation was `{"affinityMask":4095,"singleProcessorGroup":true,"processorGroup":0}` and the child reported the same, so the mask-equality assertion ran rather than being skipped. Explicitly not verified: the failing multi-group path itself. This host has 12 logical processors, which is one processor group, so the rejected-mask branch cannot occur here and only the unchanged single-group path is exercised. The reasoning for the multi-group branch comes from the Win32 contract for [processor groups](https://learn.microsoft.com/en-us/windows/win32/procthread/processor-groups) and `GetProcessGroupAffinity`, plus the experimental analysis in #2181. Confirmation on a host whose processors span more than one group — @hzh666kevin-hue's 88-processor, 2-group machine would do it — is still wanted, and the new assertion is written to cover exactly that case. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:03:19 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:43:25 +00:00

[PR #5251] [MERGED] fix(mcp): describe function comparisons in behavior tool #6839

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#5080

No description provided.

Rows
Columns