fix(windows): retain native handles when close is unconfirmed (#5772)

* fix(windows): retain native handles whose close is unconfirmed

CloseHandle failure no longer drops the numeric value. A still-open handle
stays owned and can be closed again; a value missing from the handle table
is dropped and never closed again; unreadable liveness blocks another close
until a probe says the value is open.

Co-authored-by: morluto <morluto@users.noreply.github.com>

* test(process): keep scratch until inherited pipes close

Lock the execFileOutput settlement contract: an inherited-pipe descendant
holds the caller's scratch root until it exits, and destroying the readers
is not process-family completion.

Co-authored-by: morluto <morluto@users.noreply.github.com>

* fix(windows): retry admitted file closes before runtime removal

A failed private-file close stays on the runtime that opened it. Runtime
removal closes that same wrapper before the tree walk, including when the
read that opened the file has already returned.

Co-authored-by: morluto <morluto@users.noreply.github.com>

* fix(windows): resume a close after part of the owner is released

A runtime whose directory handle is already released continues with its
parent handles instead of walking that invalid root. A process whose
settlement was already observed retries process close alone, without kill
or poll against the released job.

Co-authored-by: morluto <morluto@users.noreply.github.com>

* test(process): signal only an owned inherited-pipe descendant

Skip the descendant SIGKILL after pipe settlement. On an earlier failure,
signal only when the process command line still contains that test's path.

Co-authored-by: morluto <morluto@users.noreply.github.com>

* fix(windows): do not close a handle again after a failed close

An open liveness probe can observe a reused numeric value. After CloseHandle
fails, drop the value only when the handle table no longer contains it, and
do not call CloseHandle on that number again.

Co-authored-by: morluto <morluto@users.noreply.github.com>

* fix(windows): stop using a close-blocked pipe for I/O

A broken pipe whose CloseHandle fails stays owned, but its numeric value
may have been reused. Later polls and stdin writes treat that pipe as
ended instead of reading or writing the number.

Co-authored-by: morluto <morluto@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: morluto <morluto@users.noreply.github.com>
This commit is contained in:
authored and GitHub committed 2026-10-21 12:56:36 +08:00
1 parent fa8b87c470
commit 43ac3d1857
14 files changed
+1022 -43

No files matched your search

+15 -2
View File
@@ -111,8 +111,21 @@ automatically. Alternatively, build it using the Linux/MinGW lane above.
- Cleanup walks locked directory objects, removes files by handle, and unlinks
reparse entries without traversing their targets. Failed removal retains the
root and ancestor leases for retry; a partially removed runtime accepts only
cleanup, and its registry entry remains until removal succeeds. The runtime
owner retries only native close failures with Win32 sharing/lock codes 32/33,
cleanup, and its registry entry remains until removal succeeds. `CloseHandle`
is checked. A value that is still open, or whose liveness cannot be read,
stays with its runtime, process, or file owner and is not passed to
`CloseHandle` again. A process pipe left in that state is treated as ended
and is not read or written again. An open liveness probe does not prove that the object
still belongs to that owner. A value that is no longer in the process handle table is
dropped and is not closed again. An incomplete snapshot whose writer cannot
be closed stays on the runtime instead of being forgotten; source handles
that remain open after a published snapshot are closed with that runtime. An
admitted file whose close fails stays on the private runtime and is closed
again before that runtime's tree walk. After the root directory handle is
released, a later close resumes with the remaining parent handles and does
not walk from that released root. A process whose job close succeeded and
whose later handle close failed is retried with process close alone. The
runtime owner retries only native close failures with Win32 sharing/lock codes 32/33,
for at most ten attempts with a 100 ms delay between attempts. Other native
errors are returned immediately; exhausting the retry budget preserves the
runtime owner for a later explicit close. These cleanup retries do not use an
+133 -14
View File
@@ -7,11 +7,14 @@
#include <bcrypt.h>
#include <node_api.h>
#include "handle_close.hpp"
#include <algorithm>
#include <array>
#include <atomic>
#include <cstdint>
#include <memory>
#include <mutex>
#include <stdexcept>
#include <string>
#include <utility>
@@ -54,24 +57,134 @@ inline void require(bool condition, const char* constraint, const std::wstring&
}
class Handle {
HANDLE value_ = INVALID_HANDLE_VALUE;
public:
Handle() = default;
explicit Handle(HANDLE value) : value_(value) {}
~Handle() { reset(); }
OwnedHandle<HANDLE> owned_;
public:
Handle() noexcept : owned_(INVALID_HANDLE_VALUE) {}
explicit Handle(HANDLE value) noexcept : owned_(INVALID_HANDLE_VALUE) { adopt(value, false); }
~Handle() noexcept;
Handle(const Handle&) = delete;
Handle& operator=(const Handle&) = delete;
Handle(Handle&& other) noexcept : value_(other.release()) {}
Handle& operator=(Handle&& other) noexcept { reset(other.release()); return *this; }
HANDLE get() const { return value_; }
bool valid() const { return value_ != INVALID_HANDLE_VALUE && value_ != nullptr; }
HANDLE release() { return std::exchange(value_, INVALID_HANDLE_VALUE); }
void reset(HANDLE next = INVALID_HANDLE_VALUE) {
if (valid()) CloseHandle(value_);
value_ = next;
}
Handle(Handle&& other) noexcept : owned_(std::move(other.owned_)) {}
Handle& operator=(Handle&& other) noexcept;
HANDLE get() const noexcept { return owned_.get(); }
bool valid() const noexcept { return owned_.valid(); }
bool fullyReleased() const noexcept { return owned_.fullyReleased(); }
// A failed close leaves the numeric value blocked. It must not be used for
// I/O or closed again; only a not-in-table probe may drop it.
bool closeBlocked() const noexcept { return owned_.closeBlocked(); }
void adopt(HANDLE value, bool blockClose) noexcept;
HandleCloseDecision release() noexcept;
void reset(HANDLE next = INVALID_HANDLE_VALUE) noexcept;
};
struct RetainedNativeHandle {
HANDLE value;
bool blockClose;
};
struct RetainedNativeHandles {
std::mutex mutex;
std::vector<RetainedNativeHandle> handles;
};
// Leaked for the process lifetime. Handle destructors can run during exit,
// after an ordinary function-local static would already have been destroyed.
inline RetainedNativeHandles& retainedNativeHandles() {
static auto* const retained = new RetainedNativeHandles();
return *retained;
}
inline HANDLE normalizedNativeHandle(HANDLE value) noexcept {
return value == nullptr ? INVALID_HANDLE_VALUE : value;
}
inline CloseAttempt closeNativeHandle(HANDLE value) noexcept {
if (CloseHandle(value) != FALSE) return {true, 0};
return {false, GetLastError()};
}
inline LivenessProbe probeNativeHandle(HANDLE value) noexcept {
DWORD flags = 0;
if (GetHandleInformation(value, &flags) != FALSE) return {HandleLiveness::Open, 0};
const DWORD error = GetLastError();
if (error == ERROR_INVALID_HANDLE) return {HandleLiveness::NotInTable, error};
return {HandleLiveness::Unavailable, error};
}
// Last owner for a value whose C++ handle is dying. A failed record keeps the
// kernel object until process exit instead of closing an unconfirmed number.
inline void retainNativeHandle(HANDLE value, bool blockClose) noexcept {
value = normalizedNativeHandle(value);
if (value == INVALID_HANDLE_VALUE) return;
try {
auto& retained = retainedNativeHandles();
const std::lock_guard<std::mutex> lock(retained.mutex);
retained.handles.push_back(RetainedNativeHandle{value, blockClose});
} catch (...) {
// The number stays open until process exit. Closing it here could reuse it.
}
}
inline void Handle::adopt(HANDLE value, bool blockClose) noexcept {
value = normalizedNativeHandle(value);
if (value == INVALID_HANDLE_VALUE) return;
if (!owned_.park(value, blockClose)) retainNativeHandle(value, blockClose);
}
inline HandleCloseDecision Handle::release() noexcept {
return owned_.release(&closeNativeHandle, &probeNativeHandle);
}
inline void Handle::reset(HANDLE next) noexcept {
owned_.reset(normalizedNativeHandle(next), &closeNativeHandle, &probeNativeHandle,
&retainNativeHandle);
}
inline Handle::~Handle() noexcept {
owned_.abandon(&closeNativeHandle, &probeNativeHandle, &retainNativeHandle);
}
inline Handle& Handle::operator=(Handle&& other) noexcept {
if (this == &other) return *this;
const auto incoming = other.owned_.take();
owned_.release(&closeNativeHandle, &probeNativeHandle);
const auto keep = [&](HANDLE value, bool blockClose) noexcept {
if (value == INVALID_HANDLE_VALUE || value == nullptr) return;
if (!owned_.park(value, blockClose)) retainNativeHandle(value, blockClose);
};
keep(incoming.value, incoming.blockValue);
keep(incoming.extra, incoming.blockExtra);
return *this;
}
inline void requireReleased(Handle& handle, const char* constraint,
const std::wstring& path = L"") {
const HandleCloseDecision decision = handle.release();
if (handle.fullyReleased()) return;
const DWORD code = decision.closeError != 0 ? decision.closeError
: decision.evidenceError != 0 ? decision.evidenceError
: ERROR_GEN_FAILURE;
throw Failure(constraint, path, code);
}
// Closes values detached by a dying owner. Survivors stay in the retainer.
inline void retryRetainedNativeHandles() noexcept {
std::vector<RetainedNativeHandle> pending;
try {
auto& retained = retainedNativeHandles();
const std::lock_guard<std::mutex> lock(retained.mutex);
pending.swap(retained.handles);
} catch (...) {
return;
}
for (const RetainedNativeHandle& item : pending) {
// The destructor releases the value once and parks it again on failure.
Handle handle;
handle.adopt(item.value, item.blockClose);
}
}
enum class Kind { File, Runtime, Process };
struct Resource {
const Kind kind;
@@ -91,7 +204,13 @@ struct Runtime : Resource {
std::vector<Handle> parents;
Handle directory;
std::vector<Handle> immutableFiles;
// Handles whose close stayed unconfirmed after snapshot or owner teardown.
// runtime close retries these before deleting the private tree.
std::vector<Handle> unreleased;
bool closing = false;
// The private tree and root directory handle have been released. Later
// closes resume with the parent handles and do not duplicate that root.
bool treeReleased = false;
// Main-thread admission is single-flight; only the accepted copy owns the
// atomic cancellation flag consumed by its async worker. ERROR_BUSY rejects
// another copy before it can reset that flag or replace pending ownership.
+52 -4
View File
@@ -482,14 +482,31 @@ static void dispose(HANDLE handle, const std::wstring& path) {
require(SetFileInformationByHandle(handle, FileDispositionInfo, &deletion, sizeof(deletion)), "Delete owned object by handle failed", path);
}
static void parkUnresolved(Runtime& root, Handle& handle) noexcept {
if (handle.fullyReleased()) return;
handle.release();
if (handle.fullyReleased()) return;
try {
root.unreleased.push_back(std::move(handle));
} catch (...) {
// The owning Handle remains live. Its destructor retains the same value.
}
}
static void parkUnresolved(Runtime& root, std::vector<Handle>& handles) noexcept {
for (auto& handle : handles) parkUnresolved(root, handle);
handles.clear();
}
static void disposeSnapshot(Runtime& root, Handle& file, const std::wstring& path) {
// Snapshot readers do not share DELETE access. Release the writer only for
// failed-copy cleanup, with its parent pinned and its identity recorded.
// A failed close keeps this writer; opening a second handle would drop it.
auto parent = runtimeDirectory(root, path.substr(0, path.find_last_of(L'\\')));
FILE_ID_INFO expected{};
require(GetFileInformationByHandleEx(file.get(), FileIdInfo, &expected, sizeof(expected)),
"Read incomplete snapshot identity failed", path);
file.reset();
requireReleased(file, "Close incomplete snapshot writer failed", path);
auto cleanup = openRelative(parent->get(), path.substr(path.find_last_of(L'\\') + 1),
DELETE | GENERIC_READ | FILE_WRITE_ATTRIBUTES,
FILE_SHARE_READ, FILE_NON_DIRECTORY_FILE, path);
@@ -505,12 +522,24 @@ static void disposeSnapshot(Runtime& root, Handle& file, const std::wstring& pat
}
void closeRuntime(Runtime& root) {
retryRetainedNativeHandles();
if (root.closed) return;
root.closing = true;
// Windows keeps the bearer descriptor and snapshot leases until this owner
// cleanup. POSIX removes its consumed descriptor earlier. Abrupt owner death
// closes process jobs but cannot run this walk, so private files can remain.
for (auto& handle : root.unreleased)
requireReleased(handle, "Close retained runtime handle failed", root.path);
root.unreleased.clear();
// Writers share only FILE_SHARE_READ, which blocks a later delete open.
// Release them before the walk. A failed close stays on this owner and does
// not open a second delete handle.
for (auto& handle : root.immutableFiles)
requireReleased(handle, "Close retained runtime file failed", root.path);
root.immutableFiles.clear();
// Once the root directory handle is released, another walk would duplicate
// an invalid handle and never reach surviving parents.
if (!root.treeReleased) {
// Enumerate and open children through the retained directory handles. Paths
// below are diagnostic labels only: renames and ancestor junction replacement
// cannot redirect cleanup into an unrelated tree.
@@ -567,6 +596,7 @@ void closeRuntime(Runtime& root) {
std::wstring name;
if (!frame.next(name)) {
dispose(frame.directory.get(), frame.path);
requireReleased(frame.directory, "Close runtime directory handle failed", frame.path);
frames.pop_back();
continue;
}
@@ -578,10 +608,17 @@ void closeRuntime(Runtime& root) {
require(GetFileInformationByHandleEx(child.get(), FileAttributeTagInfo, &info, sizeof(info)), "Read cleanup object tag failed", path);
if ((info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 && (info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0)
frames.push_back(std::make_unique<Frame>(std::move(child), path));
else dispose(child.get(), path);
else {
dispose(child.get(), path);
requireReleased(child, "Close runtime child handle failed", path);
}
}
// Failed walks retain the root's identity leases for a later cleanup retry.
root.directory.reset();
requireReleased(root.directory, "Close runtime root handle failed", root.path);
root.treeReleased = true;
}
for (auto& parent : root.parents)
requireReleased(parent, "Close runtime parent handle failed", root.path);
root.parents.clear();
root.closed = true;
}
@@ -657,12 +694,17 @@ static void completeSnapshot(napi_env env, napi_status status, void* pointer) {
set(env, result, "source", identity(env, work->source->get(), work->source->requestedPath));
set(env, result, "snapshot", identity(env, work->destination.get(), work->path));
work->root->immutableFiles.push_back(std::move(work->destination));
// The digest is the byte outcome. Source handles that fail close stay on
// the runtime and are retried by runtime close; they do not erase the copy.
if (work->source != nullptr) parkUnresolved(*work->root, work->source->handles);
check(napi_resolve_deferred(env, work->deferred, result));
} catch (const Failure& failure) {
reject(failure);
} catch (const std::exception& failure) {
reject(Failure(failure.what(), work->path, ERROR_GEN_FAILURE));
}
if (work->source != nullptr) parkUnresolved(*work->root, work->source->handles);
parkUnresolved(*work->root, work->destination);
napi_delete_reference(env, work->rootReference);
napi_delete_async_work(env, work->work);
}
@@ -707,8 +749,14 @@ napi_value filesystemCall(napi_env env, const std::wstring& operation,
return result;
}
if (operation == L"close") {
retryRetainedNativeHandles();
auto& file = static_cast<File&>(resource(env, args[0], Kind::File));
file.handles.clear(); file.closed = true;
// Leave a failed handle on this File. The private runtime keeps that
// wrapper and retries this close before runtime_close.
for (auto& handle : file.handles)
requireReleased(handle, "Close admitted file handle failed", file.path);
file.handles.clear();
file.closed = true;
return null(env);
}
if (operation == L"read") {
+206
View File
@@ -0,0 +1,206 @@
#pragma once
#include <cstdint>
#include <utility>
namespace rea {
// A failed CloseHandle and a later liveness probe are not one atomic fact.
// Another thread can reuse the numeric value before GetHandleInformation
// runs, so an Open result does not prove the probed object is still ours.
// After a failed close, drop the value only when the probe says it is gone.
// Otherwise keep it and do not call CloseHandle again.
enum class HandleLiveness { Open, NotInTable, Unavailable };
enum class HandleCloseDisposition {
Released,
TerminalInvalid,
Unconfirmed,
};
struct CloseAttempt {
bool succeeded = false;
std::uint32_t error = 0;
};
struct LivenessProbe {
HandleLiveness liveness = HandleLiveness::Unavailable;
std::uint32_t error = 0;
};
struct HandleCloseDecision {
HandleCloseDisposition disposition = HandleCloseDisposition::Released;
bool clearValue = false;
bool retryClose = false;
std::uint32_t closeError = 0;
std::uint32_t evidenceError = 0;
};
inline HandleCloseDecision decideOwnedHandleClose(bool closeSucceeded,
std::uint32_t closeError,
LivenessProbe evidence) {
if (closeSucceeded)
return {HandleCloseDisposition::Released, true, false, 0, 0};
switch (evidence.liveness) {
case HandleLiveness::Open:
case HandleLiveness::Unavailable:
return {HandleCloseDisposition::Unconfirmed, false, false, closeError,
evidence.error};
case HandleLiveness::NotInTable:
return {HandleCloseDisposition::TerminalInvalid, true, false, closeError,
evidence.error};
}
return {HandleCloseDisposition::Unconfirmed, false, false, closeError,
evidence.error};
}
// One current value plus at most one parked replacement. Close and liveness
// are injected so this gate can be tested without a kernel handle table.
template <class T>
class OwnedHandle {
public:
explicit OwnedHandle(T invalid) noexcept
: invalid_(invalid), value_(invalid), extra_(invalid) {}
OwnedHandle(const OwnedHandle&) = delete;
OwnedHandle& operator=(const OwnedHandle&) = delete;
OwnedHandle(OwnedHandle&& other) noexcept
: invalid_(other.invalid_),
value_(std::exchange(other.value_, other.invalid_)),
extra_(std::exchange(other.extra_, other.invalid_)),
blockValue_(std::exchange(other.blockValue_, false)),
blockExtra_(std::exchange(other.blockExtra_, false)) {}
OwnedHandle& operator=(OwnedHandle&&) = delete;
T get() const noexcept { return value_; }
T invalid() const noexcept { return invalid_; }
bool valid() const noexcept { return value_ != invalid_; }
bool fullyReleased() const noexcept {
return value_ == invalid_ && extra_ == invalid_;
}
bool closeBlocked() const noexcept { return blockValue_; }
// Install a value into an empty slot. A live owner uses reset.
bool park(T value, bool blockClose) noexcept {
if (value == invalid_) return true;
if (value_ == invalid_) {
value_ = value;
blockValue_ = blockClose;
return true;
}
if (extra_ == invalid_) {
extra_ = value;
blockExtra_ = blockClose;
return true;
}
return false;
}
struct Slots {
T value;
T extra;
bool blockValue;
bool blockExtra;
};
Slots take() noexcept {
Slots slots{value_, extra_, blockValue_, blockExtra_};
value_ = invalid_;
extra_ = invalid_;
blockValue_ = false;
blockExtra_ = false;
return slots;
}
template <class Close, class Probe>
HandleCloseDecision release(Close&& close, Probe&& probe) noexcept {
HandleCloseDecision last{HandleCloseDisposition::Released, true, false, 0, 0};
for (;;) {
if (value_ == invalid_) {
if (extra_ == invalid_) return last;
value_ = extra_;
blockValue_ = blockExtra_;
extra_ = invalid_;
blockExtra_ = false;
}
last = finalizeSlot(value_, blockValue_, close, probe);
if (value_ != invalid_) return last;
}
}
// Close the current owner. On an unconfirmed close, keep it and park `next`
// instead of overwriting the numeric value. `sink` receives a value that
// does not fit in the parked slot; it must retain that exact value.
template <class Close, class Probe, class Sink>
void reset(T next, Close&& close, Probe&& probe, Sink&& sink) noexcept {
release(close, probe);
if (!fullyReleased()) {
if (next != invalid_ && next != value_ && next != extra_ && !park(next, false))
sink(next, false);
return;
}
value_ = next;
blockValue_ = false;
}
// Last-resort detach. Release once, then hand any surviving values to `sink`
// without a second close of a value whose liveness is not Open.
template <class Close, class Probe, class Sink>
void abandon(Close&& close, Probe&& probe, Sink&& sink) noexcept {
release(close, probe);
if (value_ != invalid_) sink(value_, blockValue_);
if (extra_ != invalid_) sink(extra_, blockExtra_);
value_ = invalid_;
extra_ = invalid_;
blockValue_ = false;
blockExtra_ = false;
}
private:
T invalid_;
T value_;
T extra_;
bool blockValue_ = false;
bool blockExtra_ = false;
template <class Close, class Probe>
HandleCloseDecision finalizeSlot(T& slot, bool& block, Close&& close,
Probe&& probe) noexcept {
if (slot == invalid_)
return {HandleCloseDisposition::Released, true, false, 0, 0};
if (block) {
const LivenessProbe evidence = probe(slot);
if (evidence.liveness == HandleLiveness::NotInTable) {
slot = invalid_;
block = false;
return {HandleCloseDisposition::TerminalInvalid, true, false, 0,
evidence.error};
}
// Open or unreadable: the number stays close-blocked. Closing it could
// release an object that reused the value.
return {HandleCloseDisposition::Unconfirmed, false, false, 0,
evidence.error};
}
const CloseAttempt attempt = close(slot);
if (attempt.succeeded) {
slot = invalid_;
block = false;
return {HandleCloseDisposition::Released, true, false, 0, 0};
}
const LivenessProbe evidence = probe(slot);
const HandleCloseDecision decision =
decideOwnedHandleClose(false, attempt.error, evidence);
if (decision.clearValue) {
slot = invalid_;
block = false;
} else {
block = !decision.retryClose;
}
return decision;
}
};
} // namespace rea
+254
View File
@@ -0,0 +1,254 @@
#include "handle_close.hpp"
#include <cstdlib>
#include <iostream>
#include <string>
#include <utility>
#include <vector>
namespace {
int failures = 0;
void expect(bool condition, const char* description) {
if (condition) return;
++failures;
std::cerr << "failed: " << description << '\n';
}
rea::CloseAttempt fail(std::uint32_t error) {
return {false, error};
}
rea::LivenessProbe live(rea::HandleLiveness liveness, std::uint32_t error = 0) {
return {liveness, error};
}
void testDecisionMatrix() {
const auto released = rea::decideOwnedHandleClose(true, 5, live(rea::HandleLiveness::Open, 9));
expect(released.disposition == rea::HandleCloseDisposition::Released && released.clearValue &&
!released.retryClose && released.closeError == 0,
"a successful close clears the value and does not retry");
const auto still = rea::decideOwnedHandleClose(false, 6, live(rea::HandleLiveness::Open));
expect(still.disposition == rea::HandleCloseDisposition::Unconfirmed && !still.clearValue &&
!still.retryClose && still.closeError == 6,
"an open probe after a failed close does not authorize another close");
const auto gone = rea::decideOwnedHandleClose(false, 6, live(rea::HandleLiveness::NotInTable, 6));
expect(gone.disposition == rea::HandleCloseDisposition::TerminalInvalid && gone.clearValue &&
!gone.retryClose && gone.evidenceError == 6,
"a value missing from the handle table is dropped and never closed again");
const auto unknown =
rea::decideOwnedHandleClose(false, 5, live(rea::HandleLiveness::Unavailable, 12));
expect(unknown.disposition == rea::HandleCloseDisposition::Unconfirmed && !unknown.clearValue &&
!unknown.retryClose && unknown.closeError == 5 && unknown.evidenceError == 12,
"unavailable liveness keeps the value and blocks another close");
}
void testReleaseRetainsAndRetriesOnlyLiveValues() {
rea::OwnedHandle<int> handle(-1);
expect(handle.park(4, false), "adopt the first value");
int closes = 0;
int probes = 0;
const auto decision = handle.release(
[&](int) {
++closes;
return fail(6);
},
[&](int) {
++probes;
return live(rea::HandleLiveness::Open);
});
expect(decision.disposition == rea::HandleCloseDisposition::Unconfirmed && handle.get() == 4 &&
closes == 1 && probes == 1 && handle.closeBlocked(),
"a failed close of a value that still looks open keeps that value blocked");
const auto retried = handle.release(
[&](int) {
++closes;
return rea::CloseAttempt{true, 0};
},
[&](int) {
++probes;
return live(rea::HandleLiveness::Open);
});
expect(retried.disposition == rea::HandleCloseDisposition::Unconfirmed && handle.get() == 4 &&
handle.closeBlocked() && closes == 1 && probes == 2,
"a later open probe does not close a value whose previous close failed");
}
void testUnconfirmedCloseDoesNotCallCloseAgain() {
rea::OwnedHandle<int> handle(-1);
expect(handle.park(8, false), "adopt the unconfirmed value");
int closes = 0;
int probes = 0;
handle.release(
[&](int) {
++closes;
return fail(5);
},
[&](int) {
++probes;
return live(rea::HandleLiveness::Unavailable, 12);
});
expect(handle.get() == 8 && handle.closeBlocked() && closes == 1 && probes == 1,
"the first failed close records unconfirmed liveness");
handle.release(
[&](int) {
++closes;
return fail(5);
},
[&](int) {
++probes;
return live(rea::HandleLiveness::Unavailable, 12);
});
expect(closes == 1 && probes == 2 && handle.get() == 8 && handle.closeBlocked(),
"unconfirmed liveness does not call close again");
handle.release(
[&](int) {
++closes;
return rea::CloseAttempt{true, 0};
},
[&](int) {
++probes;
return live(rea::HandleLiveness::Open);
});
expect(handle.get() == 8 && handle.closeBlocked() && closes == 1 && probes == 3,
"a later open probe still does not close the unconfirmed value");
}
void testTerminalInvalidityDropsTheValue() {
rea::OwnedHandle<int> handle(-1);
expect(handle.park(3, false), "adopt the invalid value");
int closes = 0;
const auto decision = handle.release(
[&](int) {
++closes;
return fail(6);
},
[&](int) { return live(rea::HandleLiveness::NotInTable, 6); });
expect(decision.disposition == rea::HandleCloseDisposition::TerminalInvalid &&
handle.fullyReleased() && closes == 1,
"terminal invalidity clears the owner");
handle.release(
[&](int) {
++closes;
return fail(6);
},
[&](int) { return live(rea::HandleLiveness::Open); });
expect(closes == 1 && handle.fullyReleased(),
"a dropped value is not passed to close again");
}
void testBlockedProbeCanDiscoverTerminalInvalidityWithoutClosing() {
rea::OwnedHandle<int> handle(-1);
expect(handle.park(9, true), "install a close-blocked value");
int closes = 0;
const auto decision = handle.release(
[&](int) {
++closes;
return fail(1);
},
[&](int) { return live(rea::HandleLiveness::NotInTable, 6); });
expect(decision.disposition == rea::HandleCloseDisposition::TerminalInvalid &&
handle.fullyReleased() && closes == 0,
"a blocked value that is no longer in the table is not closed");
}
void testResetParksTheReplacementAndOverflowsTheThirdValue() {
rea::OwnedHandle<int> handle(-1);
expect(handle.park(1, false), "adopt the current value");
std::vector<std::pair<int, bool>> sunk;
handle.reset(
2,
[&](int) { return fail(6); },
[&](int) { return live(rea::HandleLiveness::Open); },
[&](int value, bool block) { sunk.emplace_back(value, block); });
expect(handle.get() == 1 && sunk.empty(), "a failed close does not overwrite the live value");
int closes = 0;
handle.reset(
3,
[&](int value) {
++closes;
return value == 1 ? fail(6) : rea::CloseAttempt{true, 0};
},
[&](int) { return live(rea::HandleLiveness::Open); },
[&](int value, bool block) { sunk.emplace_back(value, block); });
expect(handle.get() == 1 && handle.closeBlocked() && sunk.size() == 1 && sunk[0].first == 3 &&
!sunk[0].second && closes == 0,
"a blocked value is not closed again and a third value is retained by the sink");
}
void testReleaseClosesTheParkedReplacementAfterTheCurrentValue() {
rea::OwnedHandle<int> handle(-1);
expect(handle.park(1, false), "adopt the current value");
expect(handle.park(2, false), "park the replacement");
std::vector<int> order;
const auto decision = handle.release(
[&](int value) {
order.push_back(value);
return rea::CloseAttempt{true, 0};
},
[&](int) { return live(rea::HandleLiveness::Unavailable, 1); });
expect(decision.disposition == rea::HandleCloseDisposition::Released && handle.fullyReleased() &&
order.size() == 2 && order[0] == 1 && order[1] == 2,
"release closes the current value and then the parked replacement");
}
void testAbandonHandsSurvivingValuesToTheSinkOnce() {
rea::OwnedHandle<int> handle(-1);
expect(handle.park(7, false), "adopt the abandoned value");
expect(handle.park(8, false), "park a second abandoned value");
int closes = 0;
std::vector<std::pair<int, bool>> sunk;
handle.abandon(
[&](int) {
++closes;
return fail(5);
},
[&](int) { return live(rea::HandleLiveness::Unavailable, 11); },
[&](int value, bool block) { sunk.emplace_back(value, block); });
expect(handle.fullyReleased() && closes == 1 && sunk.size() == 2 && sunk[0] == std::make_pair(7, true) &&
sunk[1] == std::make_pair(8, false),
"abandon retains every surviving value and does not close the unconfirmed one twice");
}
void testMovePreservesTheCloseGate() {
rea::OwnedHandle<int> handle(-1);
expect(handle.park(4, true), "adopt a blocked value");
rea::OwnedHandle<int> moved(std::move(handle));
expect(handle.fullyReleased() && moved.get() == 4 && moved.closeBlocked(),
"move transfers the value and its close gate");
int closes = 0;
moved.release([&](int) {
++closes;
return fail(1);
},
[&](int) { return live(rea::HandleLiveness::Unavailable, 2); });
expect(closes == 0 && moved.get() == 4 && moved.closeBlocked(),
"a moved blocked value is probed and not closed while liveness is unknown");
}
} // namespace
int main() {
testDecisionMatrix();
testReleaseRetainsAndRetriesOnlyLiveValues();
testUnconfirmedCloseDoesNotCallCloseAgain();
testTerminalInvalidityDropsTheValue();
testBlockedProbeCanDiscoverTerminalInvalidityWithoutClosing();
testResetParksTheReplacementAndOverflowsTheThirdValue();
testReleaseClosesTheParkedReplacementAfterTheCurrentValue();
testAbandonHandsSurvivingValuesToTheSinkOnce();
testMovePreservesTheCloseGate();
if (failures != 0) {
std::cerr << failures << " assertion(s) failed\n";
return EXIT_FAILURE;
}
return EXIT_SUCCESS;
}
+21 -6
View File
@@ -217,14 +217,19 @@ static napi_value readOutput(napi_env env, Handle& read, bool& ended) {
constexpr size_t kMaximumReadPerPoll = 1024 * 1024;
std::array<unsigned char, 65536> chunk;
std::vector<unsigned char> bytes;
if (!read.valid()) ended = true;
// A close-blocked pipe is EOF. Its number may have been reused, so this
// poll must not pass it to PeekNamedPipe or ReadFile. process_close still
// owns that value.
if (!read.valid() || read.closeBlocked()) ended = true;
else {
while (bytes.size() < kMaximumReadPerPoll) {
DWORD available = 0;
if (!PeekNamedPipe(read.get(), nullptr, 0, nullptr, &available, nullptr)) {
require(GetLastError() == ERROR_BROKEN_PIPE,
"Observe owned process output failed");
read.reset();
// Broken pipe is EOF for the bytes already queued. A failed close keeps
// this handle for process_close instead of dropping those bytes.
read.release();
ended = true;
break;
}
@@ -350,7 +355,8 @@ static void completeInput(napi_env env, napi_status status, void* pointer) {
}
static napi_value inputWrite(napi_env env, napi_value owner, Process& process, napi_value input) {
require(process.stdinWrite.valid(), "Owned process has no writable input", L"", ERROR_BROKEN_PIPE);
require(process.stdinWrite.valid() && !process.stdinWrite.closeBlocked(),
"Owned process has no writable input", L"", ERROR_BROKEN_PIPE);
require(!process.inputPending, "Owned process input already has a pending write", L"", ERROR_BUSY);
auto work = std::make_unique<InputWork>(process);
void* bytes = nullptr;
@@ -411,7 +417,7 @@ napi_value processCall(napi_env env, const std::wstring& operation, const std::v
require(args.size() == 1, "Wrong native process handle argument count", L"", ERROR_INVALID_PARAMETER);
auto& process = static_cast<Process&>(resource(env, args[0], Kind::Process));
if (operation == L"process_stdin_close") {
process.stdinWrite.reset();
requireReleased(process.stdinWrite, "Close owned process stdin failed");
return null(env);
}
if (operation == L"process_poll") {
@@ -446,8 +452,17 @@ napi_value processCall(napi_env env, const std::wstring& operation, const std::v
}
if (operation == L"process_close") {
// Closing the job is the crash-safe authority; no PID enumeration is used.
process.job.reset(); process.process.reset(); process.stdoutRead.reset(); process.stderrRead.reset(); process.stdinWrite.reset();
process.notifications.reset();
// A failed close leaves that handle on this process so a later close can
// retry it. Handles that already closed stay released, so the retry
// resumes at the first handle still owned. Callers that have observed
// settlement must retry this close alone; kill and poll still need the job.
retryRetainedNativeHandles();
requireReleased(process.job, "Close owned Job Object failed");
requireReleased(process.process, "Close owned process failed");
requireReleased(process.stdoutRead, "Close owned process stdout failed");
requireReleased(process.stderrRead, "Close owned process stderr failed");
requireReleased(process.stdinWrite, "Close owned process stdin failed");
requireReleased(process.notifications, "Close owned process notifications failed");
process.closed = true;
return null(env);
}
@@ -518,6 +518,9 @@ export const createDarwinProcessRunTokenReader = (
} catch (cause: unknown) {
operation.primaryFailure = { cause };
if (operationRoot !== undefined) {
// execFileOutput has settled, so inherited compiler pipes are closed.
// That is not proof for a child that did not inherit them; this owner
// does not signal processes it did not launch.
try {
await cleanupRoot(operationRoot);
} catch (cleanup: unknown) {
+110 -1
View File
@@ -1,4 +1,5 @@
import { access, readFile, rm } from "node:fs/promises";
import { execFile } from "node:child_process";
import { access, mkdir, readFile, rm } from "node:fs/promises";
import { join } from "node:path";
import { describe, expect, it } from "vitest";
@@ -292,6 +293,114 @@ describe.skipIf(process.platform === "win32")(
},
);
const descendantCommand = async (pid: number): Promise<string> => {
if (process.platform === "linux")
return readFile(`/proc/${pid}/cmdline`, "utf8").catch(() => "");
return new Promise((resolve) => {
execFile(
"ps",
["-p", String(pid), "-o", "command="],
{ encoding: "utf8" },
(error, stdout) => {
resolve(error ? "" : stdout);
},
);
});
};
/** Signal only a still-running descendant whose command line contains `marker`. */
const stopOwnedDescendant = async (
pid: number,
marker: string,
): Promise<unknown> => {
if (!Number.isInteger(pid) || pid <= 0 || pid === process.pid)
return undefined;
if (!(await descendantCommand(pid)).includes(marker)) return undefined;
try {
process.kill(pid, "SIGKILL");
} catch (cause: unknown) {
if (cause instanceof Error && "code" in cause && cause.code === "ESRCH")
return undefined;
return cause;
}
return undefined;
};
describe.skipIf(process.platform === "win32")(
"execFileOutput inherited pipes",
() => {
it("keeps scratch files until an inherited-pipe descendant closes them", async () => {
const directory = await createTestTempDirectory("rea-exec-inherited-");
const scratch = join(directory, "scratch");
const finished = join(directory, "finished.json");
const pidPath = join(directory, "child.pid");
await mkdir(scratch);
const childSource = [
'const fs = require("node:fs");',
"setTimeout(() => {",
` fs.writeFileSync(${JSON.stringify(finished)}, JSON.stringify({ scratchExists: fs.existsSync(${JSON.stringify(scratch)}) }));`,
" process.exit(0);",
"}, 7000);",
].join("\n");
const script = [
'const { spawn } = require("node:child_process");',
'const fs = require("node:fs");',
`const child = spawn(process.execPath, ["-e", ${JSON.stringify(childSource)}], { stdio: "inherit" });`,
`fs.writeFileSync(${JSON.stringify(pidPath)}, String(child.pid));`,
"process.exit(0);",
].join("\n");
const settled = execFileOutput(process.execPath, ["-e", script], {
timeout: 200,
stopSignal: "SIGTERM",
})
.finally(async () => {
await rm(scratch, { recursive: true, force: true });
})
.then(
() => ({ ok: true as const }),
(cause: unknown) => ({ ok: false as const, cause }),
);
let killFailure: unknown;
let descendantSettled = false;
try {
const deadline = Date.now() + 15_000;
let text: string | undefined;
while (Date.now() < deadline) {
text = await readFile(finished, "utf8").then(
(value) => value,
(cause: unknown) => {
if (
cause instanceof Error &&
"code" in cause &&
cause.code === "ENOENT"
)
return undefined;
throw cause;
},
);
if (text !== undefined) break;
await new Promise((resolve) => setTimeout(resolve, 20));
}
expect(text).toBeDefined();
expect(JSON.parse(text ?? "")).toEqual({ scratchExists: true });
const outcome = await settled;
descendantSettled = true;
expect(outcome).toMatchObject({
ok: false,
cause: { code: "ETIMEDOUT", killed: true },
});
} finally {
if (!descendantSettled) {
const pid = Number(await readFile(pidPath, "utf8").catch(() => ""));
killFailure = await stopOwnedDescendant(pid, finished);
}
await settled;
}
if (killFailure !== undefined) throw killFailure;
});
},
);
describe("execFileOutput failure metadata", () => {
it("preserves raw byte counts on a failed command with invalid UTF-8", async () => {
const failure: unknown = await execFileOutput(process.execPath, [
+6 -4
View File
@@ -10,10 +10,12 @@ type ExecFileOutputOptions = Omit<
readonly maxBuffer?: number;
/**
* Signal that stops the process on abort or timeout. The promise then
* settles only after the process exits, so the caller can remove files it
* writes; a process still running after a grace period is killed. Without
* it, Node's execFile sends SIGTERM on abort and rejects before the
* process exits.
* settles only after that process's stdio pipes close, so a caller can
* remove files the command wrote. A descendant that inherited those pipes
* keeps them open until it exits; destroying the readers is not
* process-family completion and must not unblock this settlement. A process
* still running after a grace period is killed. Without this signal, Node's
* execFile sends SIGTERM on abort and rejects before the process exits.
*/
readonly stopSignal?: NodeJS.Signals;
};
+53
View File
@@ -0,0 +1,53 @@
import { execFile, spawnSync } from "node:child_process";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { promisify } from "node:util";
import { fileURLToPath } from "node:url";
import { describe, expect, it } from "vitest";
const run = promisify(execFile);
const compilerProbe = spawnSync("g++", ["--version"], { stdio: "ignore" });
const compilerMissing =
compilerProbe.error instanceof Error &&
"code" in compilerProbe.error &&
compilerProbe.error.code === "ENOENT";
describe.skipIf(process.platform === "win32" && compilerMissing)(
"Windows native handle close gate",
() => {
it("retains a value unless close succeeds or the table no longer contains it", async () => {
// Injected close and liveness results. This does not call CloseHandle.
const include = fileURLToPath(
new URL("../../native/windows/src/", import.meta.url),
);
const source = join(include, "handle_close_test.cc");
const directory = await mkdtemp(join(tmpdir(), "rea-handle-close-"));
const binary = join(directory, "handle-close-test");
try {
await run(
"g++",
[
"-std=c++17",
"-Wall",
"-Wextra",
"-Werror",
`-I${include}`,
source,
"-o",
binary,
],
{ encoding: "utf8", timeout: 60_000 },
);
const result = await run(binary, [], {
encoding: "utf8",
timeout: 30_000,
});
expect(result.stdout).toBe("");
expect(result.stderr).toBe("");
} finally {
await rm(directory, { recursive: true, force: true });
}
});
},
);
+5 -2
View File
@@ -135,10 +135,13 @@ describe("Windows native release retry", () => {
);
shouldFailClose = false;
const callsBeforeRetry = calls.length;
const cleanup = child.cleanup();
await vi.advanceTimersByTimeAsync(25);
expect(await cleanup).toMatchObject({ cleaned: true });
expect(await cleanup).toMatchObject({ cleaned: true, signaled: false });
await closed;
expect(
calls.slice(callsBeforeRetry).map(({ operation }) => operation),
).toEqual(["process_close"]);
expect(child.stdout.writableEnded).toBe(true);
expect(child.stderr.writableEnded).toBe(true);
} finally {
+39 -8
View File
@@ -65,6 +65,10 @@ export class WindowsOwnedProcess extends EventEmitter {
readonly #settled: Promise<void>;
#settle: (() => void) | undefined;
#closed = false;
// Settlement was observed and stream teardown finished, but process_close
// has not released every native handle. Later cleanup retries that close
// alone; kill and poll still need the job or process handle.
#releasePending = false;
#failure: string | undefined;
#timer: NodeJS.Timeout | undefined;
#blockedWake: NodeJS.Timeout | undefined;
@@ -247,6 +251,17 @@ export class WindowsOwnedProcess extends EventEmitter {
async #stop(): Promise<ProcessCleanupResult> {
if (this.#closed) return { cleaned: true, signaled: false };
if (this.#releasePending) {
try {
this.#release();
return { cleaned: true, signaled: false };
} catch (cause: unknown) {
return {
cleaned: false,
reason: cause instanceof Error ? cause.message : String(cause),
};
}
}
let timer: NodeJS.Timeout | undefined;
try {
const signaled = this.kill();
@@ -323,6 +338,17 @@ export class WindowsOwnedProcess extends EventEmitter {
#poll(): void {
this.#timer = undefined;
if (this.#closed) return;
if (this.#releasePending) {
try {
this.#release();
} catch (cause: unknown) {
const failure =
cause instanceof Error ? cause : new Error(String(cause));
this.#failure = failure.message;
if (this.listenerCount("error") > 0) this.emit("error", failure);
}
return;
}
try {
const observed: unknown = this.#authority.call("process_poll", [
this.#handle,
@@ -361,7 +387,8 @@ export class WindowsOwnedProcess extends EventEmitter {
// Native lifecycle failures belong to the error/cleanup channels, never
// the captured stderr observation produced by the child itself.
if (this.listenerCount("error") > 0) this.emit("error", failure);
// Retain the original job so explicit cleanup can retry its observation.
// Poll failures keep the job for another observation. A process_close
// failure leaves #releasePending so cleanup retries that close alone.
}
}
@@ -385,14 +412,18 @@ export class WindowsOwnedProcess extends EventEmitter {
#release(): void {
if (this.#closed) return;
if (this.#timer !== undefined) clearTimeout(this.#timer);
this.#clearBlockedWake();
this.stdin?.destroy();
if (!this.stdout.destroyed) this.stdout.end();
if (!this.stderr.destroyed) this.stderr.end();
// End local streams before releasing the native handle. If release fails,
// the retained job can be retried without leaving output streams open.
if (!this.#releasePending) {
if (this.#timer !== undefined) clearTimeout(this.#timer);
this.#timer = undefined;
this.#clearBlockedWake();
this.stdin?.destroy();
if (!this.stdout.destroyed) this.stdout.end();
if (!this.stderr.destroyed) this.stderr.end();
this.#releasePending = true;
}
// Streams are already ended. A failed close retries this call alone.
this.#authority.call("process_close", [this.#handle]);
this.#releasePending = false;
this.#closed = true;
this.#settle?.();
this.emit("close", this.exitCode, null);
+94
View File
@@ -151,6 +151,100 @@ describe("Windows private runtime cleanup retry", () => {
});
});
const admittedFile = {
requestedPath: "C:\\rea-private\\endpoint",
finalPath: "C:\\rea-private\\endpoint",
filesystem: "NTFS" as const,
volumeSerial: "0123456789abcdef",
fileId: "fedcba9876543210fedcba9876543210",
size: 2,
directory: false,
handle: { kind: "admitted-file" },
};
describe("Windows private runtime admitted file close", () => {
it("retries the same file wrapper before the runtime tree walk", async () => {
const failure = nativeFailure(5);
const operations: string[] = [];
let fileCloses = 0;
nativeCall.mockImplementation((operation: string) => {
operations.push(operation);
if (operation === "runtime_create") return observation;
if (operation === "runtime_open") return admittedFile;
if (operation === "read") return Buffer.from("ok");
if (operation === "close" && fileCloses++ === 0) throw failure;
return null;
});
const runtime = WindowsPrivateRuntime.create("C:\\", "rea-");
const readFailure: unknown = (() => {
try {
runtime.readFile("endpoint");
return undefined;
} catch (cause: unknown) {
return cause;
}
})();
expect(readFailure).toBe(failure);
await runtime.close();
expect(fileCloses).toBe(2);
expect(operations).toEqual([
"runtime_create",
"runtime_open",
"read",
"close",
"close",
"runtime_close",
]);
});
it("does not walk the runtime while the admitted file close is still failing", async () => {
const denied = nativeFailure(5);
const operations: string[] = [];
nativeCall.mockImplementation((operation: string) => {
operations.push(operation);
if (operation === "runtime_create") return observation;
if (operation === "runtime_open") return admittedFile;
if (operation === "read") return Buffer.from("ok");
if (operation === "close") throw denied;
return null;
});
const runtime = WindowsPrivateRuntime.create("C:\\", "rea-");
const readFailure: unknown = (() => {
try {
runtime.readFile("endpoint");
return undefined;
} catch (cause: unknown) {
return cause;
}
})();
expect(readFailure).toBe(denied);
await expect(runtime.close()).rejects.toBe(denied);
expect(operations).toEqual([
"runtime_create",
"runtime_open",
"read",
"close",
"close",
]);
expect(windowsPrivateRuntime(observation.path)).toBe(runtime);
nativeCall.mockImplementation((operation: string) => {
operations.push(operation);
if (operation === "runtime_create") return observation;
if (operation === "runtime_open") return admittedFile;
if (operation === "read") return Buffer.from("ok");
return null;
});
await runtime.close();
expect(operations.slice(-2)).toEqual(["close", "runtime_close"]);
expect(() => windowsPrivateRuntime(observation.path)).toThrow(
/No native private runtime/u,
);
});
});
it.each([995, 5])(
"classifies native snapshot code %i independently of a late abort",
async (code) => {
+31 -2
View File
@@ -63,6 +63,10 @@ export class WindowsPrivateRuntime {
#pending: Promise<unknown> | undefined;
#closed = false;
#closePromise: Promise<void> | undefined;
// Admitted files whose close failed. Removal retries these wrappers before
// the native tree walk. Dropping them leaves ancestor handles until GC, and
// those handles can make runtime_close fail with a sharing violation.
#unreleasedFiles: object[] = [];
private constructor(
readonly observation: z.infer<typeof runtimeSchema>,
private readonly authority: WindowsNativeAuthority,
@@ -151,6 +155,8 @@ export class WindowsPrivateRuntime {
relativePath,
]),
);
let text: string;
let closeFailure: unknown;
try {
const chunks: Buffer[] = [];
for (let offset = 0; offset < file.size;) {
@@ -170,10 +176,17 @@ export class WindowsPrivateRuntime {
offset += value.length;
chunks.push(value);
}
return Buffer.concat(chunks).toString("utf8");
text = Buffer.concat(chunks).toString("utf8");
} finally {
this.authority.call("close", [file.handle]);
try {
this.authority.call("close", [file.handle]);
} catch (cause: unknown) {
this.#unreleasedFiles.push(file.handle);
closeFailure = cause;
}
}
if (closeFailure !== undefined) throw closeFailure;
return text;
}
/** Remove owned objects by handle, unlinking reparse entries without traversal. */
@@ -195,9 +208,25 @@ export class WindowsPrivateRuntime {
runtimes.delete(windowsRuntimeIdentityKey(this.observation));
}
/** Close admitted files that survived readFile before walking the tree. */
#releaseAdmittedFiles(): void {
const pending = this.#unreleasedFiles.splice(0);
let failure: unknown;
for (const handle of pending) {
try {
this.authority.call("close", [handle]);
} catch (cause: unknown) {
this.#unreleasedFiles.push(handle);
failure ??= cause;
}
}
if (failure !== undefined) throw failure;
}
async #closeRuntime(): Promise<void> {
for (let attempt = 1; ; attempt++) {
try {
this.#releaseAdmittedFiles();
this.authority.call("runtime_close", [this.observation.handle]);
return;
} catch (cause: unknown) {