fix(daemon): address QA round 1

- Separate loss from transient failure. Touch helpers now report OK /
  lost / transient (version_cohort: OK/UNSAFE/IO; ipc: 1/0/-1). Only a
  proven unlink or replacement stops the daemon. A refresh failure on a
  still-valid file (EROFS, EPERM, EMFILE on the marker reopen) logs
  daemon.coordination_touch_failed once, retries every tick, and logs
  daemon.coordination_touch_recovered when it clears.
- Cohort and claim locks now prove the path still names the held inode
  (private_file_revalidate against the stored directory + name), not
  just st_nlink, so rename-away, hard-link+unlink and a replaced
  directory are caught like unlink.
- The runtime service's self-owned participant guard is refreshed by
  the listener heartbeat too.
- Tests: rename-away detection; cohort lease/claim touch (lease with
  only lifetime held is OK, each unlinked file reported independently);
  IPC listener touch now checks timestamps are actually refreshed and
  covers the participant guard and identity-marker replacement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
This commit is contained in:
Shane McCarronandClaude Opus 5 committed 2026-09-21 13:02:07 -05:00
1 parent 9e793df87b
commit 3cf2a6663f
12 files changed
+329 -129

No files matched your search

+49 -22
View File
@@ -879,26 +879,41 @@ typedef struct {
cbm_daemon_ipc_participant_guard_t *participant_guard; cbm_daemon_ipc_participant_guard_t *participant_guard;
} host_coordination_t; } host_coordination_t;
/* Refresh every long-held runtime file and name the first one found lost, or typedef struct {
* return NULL. A held lock whose path now names another inode coordinates const char *lost; /* first held file whose path no longer names it */
* nothing: forked index workers read the daemon as uncoordinated and a peer const char *transient; /* first still-valid file that could not be refreshed */
* can start a second generation (#2178). */ } host_touch_result_t;
static const char *host_coordination_touch(const host_coordination_t *coordination,
cbm_daemon_runtime_service_t *service) { static void host_touch_note(host_touch_result_t *result, const char *file, int status) {
if (!cbm_version_cohort_daemon_claim_touch(coordination->daemon_claim)) { if (status == 0 && !result->lost) {
return "cohort_daemon_claim"; result->lost = file;
} else if (status < 0 && !result->transient) {
result->transient = file;
} }
if (!cbm_version_cohort_lease_touch(coordination->cohort_lease)) { }
return "cohort_lease";
} static int host_cohort_touch_status(cbm_version_cohort_status_t status) {
if (!cbm_daemon_ipc_participant_guard_touch(coordination->endpoint, return status == CBM_VERSION_COHORT_OK ? 1 : (status == CBM_VERSION_COHORT_IO ? -1 : 0);
coordination->participant_guard)) { }
return "participant_guard";
} /* Refresh every long-held runtime file. A held lock whose path now names
if (!cbm_daemon_runtime_service_touch_listener(service)) { * another inode coordinates nothing: forked index workers read the daemon as
return "listener"; * uncoordinated and a peer can start a second generation (#2178). Every file
} * is visited even after one fails so the rest keep being refreshed. */
return NULL; static host_touch_result_t host_coordination_touch(const host_coordination_t *coordination,
cbm_daemon_runtime_service_t *service) {
host_touch_result_t result = {0};
host_touch_note(&result, "cohort_daemon_claim",
host_cohort_touch_status(
cbm_version_cohort_daemon_claim_touch(coordination->daemon_claim)));
host_touch_note(
&result, "cohort_lease",
host_cohort_touch_status(cbm_version_cohort_lease_touch(coordination->cohort_lease)));
host_touch_note(&result, "participant_guard",
cbm_daemon_ipc_participant_guard_touch(coordination->endpoint,
coordination->participant_guard));
host_touch_note(&result, "listener", cbm_daemon_runtime_service_touch_listener(service));
return result;
} }
static bool host_wait_for_lifetime(cbm_daemon_runtime_service_t *service, static bool host_wait_for_lifetime(cbm_daemon_runtime_service_t *service,
@@ -907,6 +922,7 @@ static bool host_wait_for_lifetime(cbm_daemon_runtime_service_t *service,
uint64_t initial_deadline = cbm_now_ms() + HOST_INITIAL_CLIENT_TIMEOUT_MS; uint64_t initial_deadline = cbm_now_ms() + HOST_INITIAL_CLIENT_TIMEOUT_MS;
uint64_t stopping_deadline = 0; uint64_t stopping_deadline = 0;
uint64_t next_touch = 0; uint64_t next_touch = 0;
bool touch_degraded = false;
for (;;) { for (;;) {
cbm_daemon_runtime_service_state_t state = cbm_daemon_runtime_service_state(service); cbm_daemon_runtime_service_state_t state = cbm_daemon_runtime_service_state(service);
if (state == CBM_DAEMON_RUNTIME_SERVICE_EXITED) { if (state == CBM_DAEMON_RUNTIME_SERVICE_EXITED) {
@@ -942,12 +958,23 @@ static bool host_wait_for_lifetime(cbm_daemon_runtime_service_t *service,
* coordinated generation, while re-claiming would race that client. */ * coordinated generation, while re-claiming would race that client. */
if (cbm_now_ms() >= next_touch) { if (cbm_now_ms() >= next_touch) {
next_touch = cbm_now_ms() + HOST_COORDINATION_TOUCH_MS; next_touch = cbm_now_ms() + HOST_COORDINATION_TOUCH_MS;
const char *lost = host_coordination_touch(coordination, service); host_touch_result_t touched = host_coordination_touch(coordination, service);
if (lost) { if (touched.lost) {
cbm_log_warn("daemon.lifetime_end", "reason", "coordination_file_lost", "file", cbm_log_warn("daemon.lifetime_end", "reason", "coordination_file_lost", "file",
lost); touched.lost);
return cbm_daemon_runtime_service_stop(service, HOST_RUNTIME_SHUTDOWN_MS); return cbm_daemon_runtime_service_stop(service, HOST_RUNTIME_SHUTDOWN_MS);
} }
/* A refresh failure on a still-valid file (read-only remount,
* EMFILE) is not loss: keep serving, retry next tick, and log
* only transitions so a persistent condition cannot flood. */
if ((touched.transient != NULL) != touch_degraded) {
touch_degraded = touched.transient != NULL;
if (touch_degraded) {
cbm_log_warn("daemon.coordination_touch_failed", "file", touched.transient);
} else {
cbm_log_info("daemon.coordination_touch_recovered");
}
}
} }
host_http_reconcile_at(host, cbm_now_ms(), false); host_http_reconcile_at(host, cbm_now_ms(), false);
/* Retire an ephemeral generation that lingered for cold-storm cohort /* Retire an ephemeral generation that lingered for cold-storm cohort
+66 -41
View File
@@ -3220,44 +3220,65 @@ void cbm_daemon_ipc_listener_close(cbm_daemon_ipc_listener_t *listener) {
free(listener); free(listener);
} }
/* Touch through the held descriptor only after proving the path still names /* Touch results: 1 refreshed, 0 lost (the path no longer names the owned
* it: refreshing by path would keep a replacement inode fresh and hide the * file), -1 transient (a still-valid file could not be refreshed). Touch
* loss (#2178). */ * through the held descriptor only after proving the path still names it:
static bool posix_held_file_touch(int directory_fd, const char *base_name, int fd) { * refreshing by path would keep a replacement inode fresh and hide the loss
return private_regular_file_snapshot(directory_fd, base_name, fd, 1, NULL) && * (#2178). */
futimens(fd, NULL) == 0; static int posix_touch_fold(int result, int next) {
return result == 0 || next == 0 ? 0 : (result < 0 || next < 0 ? -1 : 1);
} }
static bool posix_held_lock_touch(int directory_fd, const process_lock_entry_t *entry, int fd) { static int posix_held_file_touch(int directory_fd, const char *base_name, int fd) {
return entry && posix_held_file_touch(directory_fd, entry->lock_name, fd); if (!private_regular_file_snapshot(directory_fd, base_name, fd, 1, NULL)) {
} return 0;
bool cbm_daemon_ipc_listener_touch(cbm_daemon_ipc_listener_t *listener) {
if (!listener || listener->dir_fd < 0 || listener->owner_pid != getpid() ||
!listener->lifetime_reservation) {
return false;
} }
bool lifetime = return futimens(fd, NULL) == 0 ? 1 : -1;
posix_held_lock_touch(listener->dir_fd, listener->lifetime_reservation->process_entry, }
listener->lifetime_reservation->fd);
const cbm_daemon_ipc_participant_guard_t *guard = listener->participant_guard; static int posix_held_lock_touch(int directory_fd, const process_lock_entry_t *entry, int fd) {
bool participant = return entry ? posix_held_file_touch(directory_fd, entry->lock_name, fd) : 0;
!guard || }
posix_held_lock_touch(listener->dir_fd, guard->legacy_process_entry, guard->legacy_fd);
/* The marker is not held open; reopen it and require the published inode. */ static int posix_identity_marker_touch(const cbm_daemon_ipc_listener_t *listener) {
/* The marker is not held open; reopen it and require the published inode.
* Only ENOENT proves loss: EMFILE and friends say nothing about the file. */
int marker_fd = openat(listener->dir_fd, listener->socket_identity_name, int marker_fd = openat(listener->dir_fd, listener->socket_identity_name,
O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK); O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK);
struct stat marker_status; if (marker_fd < 0) {
bool marker = marker_fd >= 0 && return errno == ENOENT ? 0 : -1;
private_regular_file_snapshot(listener->dir_fd, listener->socket_identity_name,
marker_fd, 1, &marker_status) &&
marker_status.st_dev == listener->identity_device &&
marker_status.st_ino == listener->identity_inode &&
futimens(marker_fd, NULL) == 0;
if (marker_fd >= 0) {
(void)close(marker_fd);
} }
return lifetime && participant && marker; struct stat marker_status;
int result = 0;
if (private_regular_file_snapshot(listener->dir_fd, listener->socket_identity_name, marker_fd,
1, &marker_status) &&
marker_status.st_dev == listener->identity_device &&
marker_status.st_ino == listener->identity_inode) {
result = futimens(marker_fd, NULL) == 0 ? 1 : -1;
}
(void)close(marker_fd);
return result;
}
int cbm_daemon_ipc_listener_touch(cbm_daemon_ipc_listener_t *listener,
cbm_daemon_ipc_participant_guard_t *external_guard) {
if (!listener || listener->dir_fd < 0 || listener->owner_pid != getpid() ||
!listener->lifetime_reservation) {
return 0;
}
int result =
posix_held_lock_touch(listener->dir_fd, listener->lifetime_reservation->process_entry,
listener->lifetime_reservation->fd);
const cbm_daemon_ipc_participant_guard_t *guards[] = {listener->participant_guard,
external_guard};
for (size_t i = 0; i < sizeof(guards) / sizeof(guards[0]); i++) {
if (guards[i]) {
result = posix_touch_fold(result, posix_held_lock_touch(listener->dir_fd,
guards[i]->legacy_process_entry,
guards[i]->legacy_fd));
}
}
return posix_touch_fold(result, posix_identity_marker_touch(listener));
} }
int cbm_daemon_ipc_accept(cbm_daemon_ipc_listener_t *listener, uint32_t timeout_ms, int cbm_daemon_ipc_accept(cbm_daemon_ipc_listener_t *listener, uint32_t timeout_ms,
@@ -3643,10 +3664,12 @@ bool cbm_daemon_ipc_participant_guard_release(cbm_daemon_ipc_participant_guard_t
return true; return true;
} }
bool cbm_daemon_ipc_participant_guard_touch(const cbm_daemon_ipc_endpoint_t *endpoint, int cbm_daemon_ipc_participant_guard_touch(const cbm_daemon_ipc_endpoint_t *endpoint,
cbm_daemon_ipc_participant_guard_t *guard) { cbm_daemon_ipc_participant_guard_t *guard) {
return endpoint && guard && guard->owner_pid == getpid() && if (!endpoint || !guard || guard->owner_pid != getpid()) {
posix_held_lock_touch(endpoint->dir_fd, guard->legacy_process_entry, guard->legacy_fd); return 0;
}
return posix_held_lock_touch(endpoint->dir_fd, guard->legacy_process_entry, guard->legacy_fd);
} }
int cbm_daemon_ipc_local_transition_try_acquire( int cbm_daemon_ipc_local_transition_try_acquire(
@@ -6794,13 +6817,15 @@ bool cbm_daemon_ipc_participant_guard_release(cbm_daemon_ipc_participant_guard_t
/* Windows has no age-based cleaner of the private runtime directory, and its /* Windows has no age-based cleaner of the private runtime directory, and its
* held handles deny deletion; the #2178 heartbeat has nothing to refresh. */ * held handles deny deletion; the #2178 heartbeat has nothing to refresh. */
bool cbm_daemon_ipc_listener_touch(cbm_daemon_ipc_listener_t *listener) { int cbm_daemon_ipc_listener_touch(cbm_daemon_ipc_listener_t *listener,
return listener != NULL; cbm_daemon_ipc_participant_guard_t *external_guard) {
(void)external_guard;
return listener ? 1 : 0;
} }
bool cbm_daemon_ipc_participant_guard_touch(const cbm_daemon_ipc_endpoint_t *endpoint, int cbm_daemon_ipc_participant_guard_touch(const cbm_daemon_ipc_endpoint_t *endpoint,
cbm_daemon_ipc_participant_guard_t *guard) { cbm_daemon_ipc_participant_guard_t *guard) {
return endpoint && guard; return endpoint && guard ? 1 : 0;
} }
int cbm_daemon_ipc_local_transition_try_acquire( int cbm_daemon_ipc_local_transition_try_acquire(
+15 -11
View File
@@ -77,13 +77,17 @@ cbm_daemon_ipc_listener_t *cbm_daemon_ipc_listen_reserved(
void cbm_daemon_ipc_listener_close(cbm_daemon_ipc_listener_t *listener); void cbm_daemon_ipc_listener_close(cbm_daemon_ipc_listener_t *listener);
/* Heartbeat for the daemon's long-lived runtime artifacts: refresh the held /* Heartbeat for the daemon's long-lived runtime artifacts: refresh the held
* lifetime reservation, the listener's participant guard (if it owns one) and * lifetime reservation, the listener's participant guard (if it owns one),
* the published identity marker so an age-based temp cleaner never deletes * external_guard (optional, a guard the caller retains beside this listener)
* them (#2178). Returns false once any of them no longer names the file this * and the published identity marker so an age-based temp cleaner never
* listener owns: a lost lifetime file lets a peer start a second generation, * deletes them (#2178). Returns 1 when all were refreshed, 0 once any no
* and a lost identity marker makes listener close leave the socket pair * longer names the file this listener owns (a lost lifetime file lets a peer
* behind. Only the owning thread may call it while the listener is open. */ * start a second generation; a lost identity marker makes listener close
bool cbm_daemon_ipc_listener_touch(cbm_daemon_ipc_listener_t *listener); * leave the socket pair behind), and -1 when a still-valid file could not be
* refreshed (transient). Only the thread that closes the listener may call it
* while the listener is open. */
int cbm_daemon_ipc_listener_touch(cbm_daemon_ipc_listener_t *listener,
cbm_daemon_ipc_participant_guard_t *external_guard);
/* Create or validate one private current-user directory at an already /* Create or validate one private current-user directory at an already
* canonical local path. Ancestors are handle-validated without mutation: * canonical local path. Ancestors are handle-validated without mutation:
@@ -279,10 +283,10 @@ bool cbm_daemon_ipc_startup_lock_release(cbm_daemon_ipc_startup_lock_t **lock_io
int cbm_daemon_ipc_participant_guard_try_join(const cbm_daemon_ipc_endpoint_t *endpoint, int cbm_daemon_ipc_participant_guard_try_join(const cbm_daemon_ipc_endpoint_t *endpoint,
cbm_daemon_ipc_participant_guard_t **guard_out); cbm_daemon_ipc_participant_guard_t **guard_out);
bool cbm_daemon_ipc_participant_guard_release(cbm_daemon_ipc_participant_guard_t **guard_io); bool cbm_daemon_ipc_participant_guard_release(cbm_daemon_ipc_participant_guard_t **guard_io);
/* Same heartbeat contract as cbm_daemon_ipc_listener_touch, for a guard the /* Same heartbeat contract and 1/0/-1 result as cbm_daemon_ipc_listener_touch,
* caller retains itself. */ * for a guard the caller retains itself. */
bool cbm_daemon_ipc_participant_guard_touch(const cbm_daemon_ipc_endpoint_t *endpoint, int cbm_daemon_ipc_participant_guard_touch(const cbm_daemon_ipc_endpoint_t *endpoint,
cbm_daemon_ipc_participant_guard_t *guard); cbm_daemon_ipc_participant_guard_t *guard);
/* Standalone CLI work joins the legacy-compatible current participant group /* Standalone CLI work joins the legacy-compatible current participant group
* without becoming a daemon client. Acquisition retains startup-v2 only for * without becoming a daemon client. Acquisition retains startup-v2 only for
+4 -2
View File
@@ -2578,8 +2578,10 @@ size_t cbm_daemon_runtime_service_active_connections(cbm_daemon_runtime_service_
return count; return count;
} }
bool cbm_daemon_runtime_service_touch_listener(cbm_daemon_runtime_service_t *service) { int cbm_daemon_runtime_service_touch_listener(cbm_daemon_runtime_service_t *service) {
return service && service->listener && cbm_daemon_ipc_listener_touch(service->listener); return service && service->listener
? cbm_daemon_ipc_listener_touch(service->listener, service->owned_participant_guard)
: 0;
} }
void cbm_daemon_runtime_service_reconcile_lifetime(cbm_daemon_runtime_service_t *service) { void cbm_daemon_runtime_service_reconcile_lifetime(cbm_daemon_runtime_service_t *service) {
+6 -4
View File
@@ -336,10 +336,12 @@ size_t cbm_daemon_runtime_service_active_connections(cbm_daemon_runtime_service_
* an unbounded idle hang. The host lifetime loop calls this every tick; it is a * an unbounded idle hang. The host lifetime loop calls this every tick; it is a
* no-op unless a last-committed-client linger is armed. */ * no-op unless a last-committed-client linger is armed. */
void cbm_daemon_runtime_service_reconcile_lifetime(cbm_daemon_runtime_service_t *service); void cbm_daemon_runtime_service_reconcile_lifetime(cbm_daemon_runtime_service_t *service);
/* #2178 heartbeat over the listener's runtime artifacts; see /* #2178 heartbeat over the listener's runtime artifacts, plus the participant
* cbm_daemon_ipc_listener_touch. Call only from the thread that stops and * guard the service owns when it started itself; see
* frees the service, which is the only thread that closes the listener. */ * cbm_daemon_ipc_listener_touch for the 1/0/-1 result. Call only from the
bool cbm_daemon_runtime_service_touch_listener(cbm_daemon_runtime_service_t *service); * thread that stops and frees the service, which is the only thread that
* closes the listener. */
int cbm_daemon_runtime_service_touch_listener(cbm_daemon_runtime_service_t *service);
size_t cbm_daemon_runtime_service_job_subscribers(cbm_daemon_runtime_service_t *service, size_t cbm_daemon_runtime_service_job_subscribers(cbm_daemon_runtime_service_t *service,
const char *project_key); const char *project_key);
uint64_t cbm_daemon_runtime_service_client_process_id(cbm_daemon_runtime_service_t *service, uint64_t cbm_daemon_runtime_service_client_process_id(cbm_daemon_runtime_service_t *service,
+26 -14
View File
@@ -363,23 +363,35 @@ cbm_private_file_lock_status_t cbm_version_cohort_lease_release(
return result; return result;
} }
static bool version_cohort_lock_touch(cbm_private_file_lock_t *lock) { /* Folds one lock's touch into an accumulated result: a lost file (UNSAFE)
return !lock || cbm_private_file_lock_touch(lock) == CBM_PRIVATE_FILE_LOCK_OK; * outranks a transient refresh failure (IO), which outranks OK. A lease holds
} * only lifetime after admission, so absent locks are skipped. */
static cbm_version_cohort_status_t version_cohort_lock_touch(cbm_private_file_lock_t *lock,
bool cbm_version_cohort_lease_touch(cbm_version_cohort_lease_t *lease) { cbm_version_cohort_status_t result) {
if (!lease) { if (!lock || result == CBM_VERSION_COHORT_UNSAFE) {
return false; return result;
} }
/* Evaluate all three so one lost file does not leave the others to age. */ cbm_private_file_lock_status_t status = cbm_private_file_lock_touch(lock);
bool lifetime = version_cohort_lock_touch(lease->lifetime); if (status == CBM_PRIVATE_FILE_LOCK_UNSAFE) {
bool admission = version_cohort_lock_touch(lease->admission); return CBM_VERSION_COHORT_UNSAFE;
bool maintenance = version_cohort_lock_touch(lease->maintenance); }
return lifetime && admission && maintenance; return status == CBM_PRIVATE_FILE_LOCK_OK ? result : CBM_VERSION_COHORT_IO;
} }
bool cbm_version_cohort_daemon_claim_touch(cbm_version_cohort_daemon_claim_t *claim) { cbm_version_cohort_status_t cbm_version_cohort_lease_touch(cbm_version_cohort_lease_t *lease) {
return claim && claim->marker && version_cohort_lock_touch(claim->marker); if (!lease || !lease->lifetime) {
return CBM_VERSION_COHORT_UNSAFE;
}
cbm_version_cohort_status_t result =
version_cohort_lock_touch(lease->lifetime, CBM_VERSION_COHORT_OK);
result = version_cohort_lock_touch(lease->admission, result);
return version_cohort_lock_touch(lease->maintenance, result);
}
cbm_version_cohort_status_t cbm_version_cohort_daemon_claim_touch(
cbm_version_cohort_daemon_claim_t *claim) {
return claim && claim->marker ? version_cohort_lock_touch(claim->marker, CBM_VERSION_COHORT_OK)
: CBM_VERSION_COHORT_UNSAFE;
} }
static cbm_version_cohort_status_t version_cohort_failed(cbm_version_cohort_lease_t *lease, static cbm_version_cohort_status_t version_cohort_failed(cbm_version_cohort_lease_t *lease,
+9 -7
View File
@@ -146,13 +146,15 @@ cbm_private_file_lock_status_t cbm_version_cohort_lease_release(
cbm_version_cohort_lease_t **lease_io); cbm_version_cohort_lease_t **lease_io);
/* Heartbeat for a long-lived holder: refresh every lock file the lease or /* Heartbeat for a long-lived holder: refresh every lock file the lease or
* claim holds and report false once any held file was unlinked or replaced. * claim holds. An age-based temp cleaner deleting a held file silently ends
* An age-based temp cleaner deleting a held file silently ends coordination, * coordination, because the next participant creates and locks a fresh inode
* because the next participant creates and locks a fresh inode (#2178). The * (#2178). Returns OK when all held files were refreshed, UNSAFE once any is
* only safe response to false is an orderly exit; re-claiming would race * lost (its path no longer names the held file), and IO when a still-valid
* that participant. */ * file could not be refreshed. The only safe response to UNSAFE is an orderly
bool cbm_version_cohort_lease_touch(cbm_version_cohort_lease_t *lease); * exit; re-claiming would race that participant. IO is transient. */
bool cbm_version_cohort_daemon_claim_touch(cbm_version_cohort_daemon_claim_t *claim); cbm_version_cohort_status_t cbm_version_cohort_lease_touch(cbm_version_cohort_lease_t *lease);
cbm_version_cohort_status_t cbm_version_cohort_daemon_claim_touch(
cbm_version_cohort_daemon_claim_t *claim);
/* Refuses teardown while a lease or retryable cleanup handle remains. */ /* Refuses teardown while a lease or retryable cleanup handle remains. */
cbm_private_file_lock_status_t cbm_version_cohort_manager_free( cbm_private_file_lock_status_t cbm_version_cohort_manager_free(
+10 -3
View File
@@ -38,6 +38,9 @@ struct cbm_private_file_lock {
int fd; int fd;
pid_t owner_pid; pid_t owner_pid;
cbm_private_file_lock_mode_t mode; cbm_private_file_lock_mode_t mode;
/* Borrowed; the directory outlives every lock acquired through it. */
const cbm_private_lock_directory_t *directory;
char base_name[NAME_MAX + 1];
struct cbm_private_file_lock *next_tracked; struct cbm_private_file_lock *next_tracked;
bool unlocked; bool unlocked;
bool test_fail_unlock_once; bool test_fail_unlock_once;
@@ -345,6 +348,8 @@ cbm_private_file_lock_status_t cbm_private_file_lock_try_acquire(
lock->fd = fd; lock->fd = fd;
lock->owner_pid = getpid(); lock->owner_pid = getpid();
lock->mode = mode; lock->mode = mode;
lock->directory = directory;
memcpy(lock->base_name, base_name, strlen(base_name) + 1);
int operation = mode == CBM_PRIVATE_FILE_LOCK_SH ? LOCK_SH : LOCK_EX; int operation = mode == CBM_PRIVATE_FILE_LOCK_SH ? LOCK_SH : LOCK_EX;
if (private_flock_set(fd, operation | LOCK_NB) != 0) { if (private_flock_set(fd, operation | LOCK_NB) != 0) {
@@ -489,9 +494,11 @@ cbm_private_file_lock_status_t cbm_private_file_lock_touch(cbm_private_file_lock
if (!cbm_private_file_lock_fork_guard_enter()) { if (!cbm_private_file_lock_fork_guard_enter()) {
return CBM_PRIVATE_FILE_LOCK_IO; return CBM_PRIVATE_FILE_LOCK_IO;
} }
/* An unlinked file keeps its inode alive for this handle with st_nlink 0, /* Unlink leaves st_nlink 0 on the held inode, but a rename-away or a
* which the payload validity check already rejects. */ * replaced directory keeps it linked elsewhere; only the path-vs-handle
bool linked = private_payload_fd_valid(lock, NULL); * identity check proves the canonical path still names this lock. */
bool linked = private_payload_fd_valid(lock, NULL) && lock->directory &&
private_file_revalidate(lock->directory, lock->base_name, lock->fd, NULL);
bool touched = linked && futimens(lock->fd, NULL) == 0; bool touched = linked && futimens(lock->fd, NULL) == 0;
cbm_private_file_lock_fork_guard_leave(); cbm_private_file_lock_fork_guard_leave();
if (!linked) { if (!linked) {
+6 -3
View File
@@ -63,9 +63,12 @@ cbm_private_file_lock_status_t cbm_private_file_lock_payload_write(cbm_private_f
/* Long-lived holders call this periodically. It refreshes the held file's /* Long-lived holders call this periodically. It refreshes the held file's
* timestamps through the validated handle so age-based temp cleaners (macOS * timestamps through the validated handle so age-based temp cleaners (macOS
* tmp_cleaner, systemd-tmpfiles) never select it, and returns UNSAFE once the * tmp_cleaner, systemd-tmpfiles) never select it. Returns UNSAFE once the
* handle no longer names a linked file: a peer opening the path would then * lock's path in its directory no longer names the held file (unlinked,
* lock a different inode and the lock coordinates nothing (#2178). */ * replaced, renamed away, or the directory itself replaced): a peer opening
* the path would then lock a different inode and the lock coordinates nothing
* (#2178). IO reports a refresh failure on a still-valid lock, which callers
* should treat as transient. The directory must outlive the lock. */
cbm_private_file_lock_status_t cbm_private_file_lock_touch(cbm_private_file_lock_t *lock); cbm_private_file_lock_status_t cbm_private_file_lock_touch(cbm_private_file_lock_t *lock);
/* Forces the next successfully acquired native lock down the post-lock /* Forces the next successfully acquired native lock down the post-lock
+41 -18
View File
@@ -2064,22 +2064,34 @@ TEST(daemon_ipc_lifetime_reservation_survives_saturated_second_listen) {
} }
/* #2178: an age-based temp cleaner unlinked the live daemon's lifetime lock /* #2178: an age-based temp cleaner unlinked the live daemon's lifetime lock
* and identity marker. The heartbeat must report either loss, so the daemon * and identity marker. The heartbeat must keep every listener-owned file
* exits instead of serving under a lifetime file that coordinates nothing or * young, and report loss (unlink, or replacement by a fresh file at the same
* leaving an unremovable socket pair behind at close. */ * name) so the daemon exits instead of serving under a lifetime file that
* coordinates nothing or leaving an unremovable socket pair behind at close. */
TEST(daemon_ipc_listener_touch_detects_lost_runtime_files) { TEST(daemon_ipc_listener_touch_detects_lost_runtime_files) {
#ifdef _WIN32 #ifdef _WIN32
SKIP_PLATFORM("unlink-while-held of runtime files is POSIX behavior"); SKIP_PLATFORM("unlink-while-held of runtime files is POSIX behavior");
#else #else
enum { TOUCH_CASES = 4 };
static const char key[] = "2178a2178a2178a0"; static const char key[] = "2178a2178a2178a0";
static const char *const lost_files[] = {"lifetime.lock", "sock.identity"}; static const struct {
const char *suffix;
bool replace;
} cases[TOUCH_CASES] = {
{"lifetime.lock", false}, /* lifetime reservation */
{"lock", false}, /* listener-owned participant guard */
{"sock.identity", false}, /* identity marker, unlinked */
{"sock.identity", true}, /* identity marker, replaced by a fresh inode */
};
const time_t stale = 1000000000;
char parent[TEST_PATH_CAP] = {0}; char parent[TEST_PATH_CAP] = {0};
char runtime_dir[TEST_PATH_CAP] = {0}; char runtime_dir[TEST_PATH_CAP] = {0};
cbm_daemon_ipc_endpoint_t *endpoint = NULL; cbm_daemon_ipc_endpoint_t *endpoint = NULL;
bool started[2] = {false, false}; bool started[TOUCH_CASES] = {false};
bool touched_intact[2] = {false, false}; int touched_intact[TOUCH_CASES] = {0};
bool unlinked[2] = {false, false}; bool refreshed[TOUCH_CASES] = {false};
bool touched_after_loss[2] = {true, true}; bool lost[TOUCH_CASES] = {false};
int touched_after_loss[TOUCH_CASES] = {1, 1, 1, 1};
if (ipc_test_parent_new(parent, "listener-touch")) { if (ipc_test_parent_new(parent, "listener-touch")) {
endpoint = cbm_daemon_ipc_endpoint_new(key, parent); endpoint = cbm_daemon_ipc_endpoint_new(key, parent);
@@ -2087,25 +2099,36 @@ TEST(daemon_ipc_listener_touch_detects_lost_runtime_files) {
if (endpoint) { if (endpoint) {
ipc_test_copy_path(runtime_dir, cbm_daemon_ipc_endpoint_runtime_dir(endpoint)); ipc_test_copy_path(runtime_dir, cbm_daemon_ipc_endpoint_runtime_dir(endpoint));
} }
for (size_t i = 0; endpoint && i < 2; i++) { for (size_t i = 0; endpoint && i < TOUCH_CASES; i++) {
cbm_daemon_ipc_listener_t *listener = cbm_daemon_ipc_listen(endpoint); cbm_daemon_ipc_listener_t *listener = cbm_daemon_ipc_listen(endpoint);
started[i] = listener != NULL; started[i] = listener != NULL;
touched_intact[i] = started[i] && cbm_daemon_ipc_listener_touch(listener);
char path[TEST_PATH_CAP]; char path[TEST_PATH_CAP];
int written = snprintf(path, sizeof(path), "%s/cbm-%s.%s", runtime_dir, key, lost_files[i]); int written =
unlinked[i] = snprintf(path, sizeof(path), "%s/cbm-%s.%s", runtime_dir, key, cases[i].suffix);
touched_intact[i] && written > 0 && written < (int)sizeof(path) && unlink(path) == 0; bool path_ok = started[i] && written > 0 && written < (int)sizeof(path);
touched_after_loss[i] = unlinked[i] && cbm_daemon_ipc_listener_touch(listener); struct timespec stale_times[2] = {{.tv_sec = stale}, {.tv_sec = stale}};
bool aged = path_ok && utimensat(AT_FDCWD, path, stale_times, AT_SYMLINK_NOFOLLOW) == 0;
touched_intact[i] = aged ? cbm_daemon_ipc_listener_touch(listener, NULL) : -2;
struct stat after;
refreshed[i] = touched_intact[i] == 1 && stat(path, &after) == 0 &&
after.st_atime > stale && after.st_mtime > stale;
lost[i] = refreshed[i] && unlink(path) == 0;
if (lost[i] && cases[i].replace) {
int fd = open(path, O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW, 0600);
lost[i] = fd >= 0 && close(fd) == 0;
}
touched_after_loss[i] = lost[i] ? cbm_daemon_ipc_listener_touch(listener, NULL) : -2;
cbm_daemon_ipc_listener_close(listener); cbm_daemon_ipc_listener_close(listener);
} }
cbm_daemon_ipc_endpoint_free(endpoint); cbm_daemon_ipc_endpoint_free(endpoint);
ipc_test_remove_tree(runtime_dir, parent); ipc_test_remove_tree(runtime_dir, parent);
for (size_t i = 0; i < 2; i++) { for (size_t i = 0; i < TOUCH_CASES; i++) {
ASSERT_TRUE(started[i]); ASSERT_TRUE(started[i]);
ASSERT_TRUE(touched_intact[i]); ASSERT_EQ(touched_intact[i], 1);
ASSERT_TRUE(unlinked[i]); ASSERT_TRUE(refreshed[i]);
ASSERT_FALSE(touched_after_loss[i]); ASSERT_TRUE(lost[i]);
ASSERT_EQ(touched_after_loss[i], 0);
} }
PASS(); PASS();
#endif #endif
+46 -1
View File
@@ -167,7 +167,9 @@ static void private_lock_fixture_finish(private_lock_fixture_t *fixture) {
"acl-directory.lock", "acl-directory.lock",
"acl-file.lock", "acl-file.lock",
"fork.lock", "fork.lock",
"touch.lock"}; "touch.lock",
"touch-rename.lock",
"touch-renamed.lock"};
if (fixture->root[0]) { if (fixture->root[0]) {
for (size_t index = 0; index < sizeof(files) / sizeof(files[0]); index++) { for (size_t index = 0; index < sizeof(files) / sizeof(files[0]); index++) {
if (!private_lock_path(path, fixture, files[index])) { if (!private_lock_path(path, fixture, files[index])) {
@@ -540,6 +542,48 @@ TEST(private_file_lock_touch_refreshes_held_file_and_detects_unlink) {
#endif #endif
} }
/* A rename-away keeps the held inode linked (st_nlink stays 1) while the
* canonical path no longer names it, so a peer would create and lock a fresh
* file there. Touch must compare the path with the handle, not just count
* links. */
TEST(private_file_lock_touch_detects_rename_away) {
#ifdef _WIN32
SKIP_PLATFORM("renaming a held lock file away is POSIX behavior");
#else
private_lock_fixture_t fixture;
bool started = private_lock_fixture_start(&fixture);
char path[PRIVATE_LOCK_TEST_PATH_CAP];
char moved[PRIVATE_LOCK_TEST_PATH_CAP];
bool paths_ok = started && private_lock_path(path, &fixture, "touch-rename.lock") &&
private_lock_path(moved, &fixture, "touch-renamed.lock");
cbm_private_file_lock_t *held = NULL;
cbm_private_file_lock_status_t acquired =
paths_ok ? cbm_private_file_lock_try_acquire(fixture.directory, "touch-rename.lock",
CBM_PRIVATE_FILE_LOCK_EX, &held)
: CBM_PRIVATE_FILE_LOCK_IO;
cbm_private_file_lock_status_t intact_touch = acquired == CBM_PRIVATE_FILE_LOCK_OK
? cbm_private_file_lock_touch(held)
: CBM_PRIVATE_FILE_LOCK_IO;
bool renamed = intact_touch == CBM_PRIVATE_FILE_LOCK_OK && rename(path, moved) == 0;
struct stat moved_status = {0};
bool still_linked = renamed && stat(moved, &moved_status) == 0 && moved_status.st_nlink == 1;
cbm_private_file_lock_status_t renamed_touch =
renamed ? cbm_private_file_lock_touch(held) : CBM_PRIVATE_FILE_LOCK_IO;
if (held) {
(void)cbm_private_file_lock_release(&held);
}
private_lock_fixture_finish(&fixture);
ASSERT_TRUE(started);
ASSERT_EQ(acquired, CBM_PRIVATE_FILE_LOCK_OK);
ASSERT_EQ(intact_touch, CBM_PRIVATE_FILE_LOCK_OK);
ASSERT_TRUE(renamed);
ASSERT_TRUE(still_linked);
ASSERT_EQ(renamed_touch, CBM_PRIVATE_FILE_LOCK_UNSAFE);
PASS();
#endif
}
TEST(private_file_lock_post_acquire_failure_returns_cleanup_owner) { TEST(private_file_lock_post_acquire_failure_returns_cleanup_owner) {
private_lock_fixture_t fixture; private_lock_fixture_t fixture;
bool started = private_lock_fixture_start(&fixture); bool started = private_lock_fixture_start(&fixture);
@@ -924,6 +968,7 @@ SUITE(private_file_lock) {
RUN_TEST(private_file_lock_close_failure_retries_without_duplicate_unlock); RUN_TEST(private_file_lock_close_failure_retries_without_duplicate_unlock);
RUN_TEST(private_file_lock_consumed_close_error_never_retries_recycled_fd); RUN_TEST(private_file_lock_consumed_close_error_never_retries_recycled_fd);
RUN_TEST(private_file_lock_touch_refreshes_held_file_and_detects_unlink); RUN_TEST(private_file_lock_touch_refreshes_held_file_and_detects_unlink);
RUN_TEST(private_file_lock_touch_detects_rename_away);
RUN_TEST(private_file_lock_post_acquire_failure_returns_cleanup_owner); RUN_TEST(private_file_lock_post_acquire_failure_returns_cleanup_owner);
RUN_TEST(private_file_lock_windows_lock_attempt_failure_returns_cleanup_owner); RUN_TEST(private_file_lock_windows_lock_attempt_failure_returns_cleanup_owner);
RUN_TEST(private_file_lock_rejects_unsafe_entries_and_replaced_root); RUN_TEST(private_file_lock_rejects_unsafe_entries_and_replaced_root);
+51 -3
View File
@@ -330,9 +330,9 @@ TEST(version_cohort_conflict_is_retried_until_the_deadline) {
/* An indefinite deadline never waits on a conflicting holder. */ /* An indefinite deadline never waits on a conflicting holder. */
retries_before = cbm_version_cohort_conflict_retries_for_testing(); retries_before = cbm_version_cohort_conflict_retries_for_testing();
ASSERT_EQ(cbm_version_cohort_acquire(second, &requested, UINT64_MAX, &requested_lease, ASSERT_EQ(
&conflict), cbm_version_cohort_acquire(second, &requested, UINT64_MAX, &requested_lease, &conflict),
CBM_VERSION_COHORT_CONFLICT); CBM_VERSION_COHORT_CONFLICT);
ASSERT_NULL(requested_lease); ASSERT_NULL(requested_lease);
ASSERT_EQ(cbm_version_cohort_conflict_retries_for_testing(), retries_before); ASSERT_EQ(cbm_version_cohort_conflict_retries_for_testing(), retries_before);
@@ -669,6 +669,53 @@ TEST(version_cohort_does_not_repurpose_daemon_startup_lock_for_lifetime) {
PASS(); PASS();
} }
/* #2178: a live daemon's lease holds only the lifetime file after admission
* (admission and maintenance are released), and its claim holds the marker.
* Touch must report both intact, then name each as lost once an age-based
* temp cleaner unlinks it, independently of the other. */
TEST(version_cohort_touch_reports_each_lost_file) {
#ifdef _WIN32
SKIP_PLATFORM("unlinking a held lock file is POSIX behavior");
#else
version_cohort_fixture_t fixture;
ASSERT_TRUE(version_cohort_fixture_start(&fixture, "touch"));
cbm_version_cohort_manager_t *manager = cbm_version_cohort_manager_new(fixture.endpoint);
ASSERT_NOT_NULL(manager);
cbm_daemon_build_identity_t identity = version_cohort_identity("1.0.0", VERSION_COHORT_BUILD_A);
cbm_daemon_conflict_t conflict;
cbm_version_cohort_lease_t *lease = NULL;
cbm_version_cohort_daemon_claim_t *claim = NULL;
ASSERT_EQ(
cbm_version_cohort_acquire(manager, &identity, cbm_now_ms() + 5000, &lease, &conflict),
CBM_VERSION_COHORT_OK);
ASSERT_EQ(cbm_version_cohort_daemon_claim_acquire(manager, &claim), CBM_VERSION_COHORT_OK);
ASSERT_EQ(cbm_version_cohort_lease_touch(lease), CBM_VERSION_COHORT_OK);
ASSERT_EQ(cbm_version_cohort_daemon_claim_touch(claim), CBM_VERSION_COHORT_OK);
const char *runtime_dir = cbm_daemon_ipc_endpoint_runtime_dir(fixture.endpoint);
char marker[VERSION_COHORT_TEST_PATH_CAP];
char lifetime[VERSION_COHORT_TEST_PATH_CAP];
ASSERT_TRUE(snprintf(marker, sizeof(marker), "%s/cbm-version-cohort-daemon-v1.lock",
runtime_dir) < (int)sizeof(marker));
ASSERT_TRUE(snprintf(lifetime, sizeof(lifetime), "%s/cbm-version-cohort-lifetime-v1.lock",
runtime_dir) < (int)sizeof(lifetime));
ASSERT_EQ(unlink(marker), 0);
ASSERT_EQ(cbm_version_cohort_daemon_claim_touch(claim), CBM_VERSION_COHORT_UNSAFE);
ASSERT_EQ(cbm_version_cohort_lease_touch(lease), CBM_VERSION_COHORT_OK);
ASSERT_EQ(unlink(lifetime), 0);
ASSERT_EQ(cbm_version_cohort_lease_touch(lease), CBM_VERSION_COHORT_UNSAFE);
ASSERT_EQ(cbm_version_cohort_daemon_claim_release(&claim), CBM_PRIVATE_FILE_LOCK_OK);
version_cohort_release(&lease);
version_cohort_manager_close(&manager);
version_cohort_fixture_finish(&fixture);
PASS();
#endif
}
TEST(version_cohort_distinguishes_coordinated_daemon_without_connecting) { TEST(version_cohort_distinguishes_coordinated_daemon_without_connecting) {
version_cohort_fixture_t fixture; version_cohort_fixture_t fixture;
ASSERT_TRUE(version_cohort_fixture_start(&fixture, "daemon-marker")); ASSERT_TRUE(version_cohort_fixture_start(&fixture, "daemon-marker"));
@@ -1034,6 +1081,7 @@ SUITE(version_cohort) {
RUN_TEST(version_cohort_mutation_waits_for_every_lifetime_participant); RUN_TEST(version_cohort_mutation_waits_for_every_lifetime_participant);
RUN_TEST(version_cohort_mutation_timeout_releases_all_guards); RUN_TEST(version_cohort_mutation_timeout_releases_all_guards);
RUN_TEST(version_cohort_does_not_repurpose_daemon_startup_lock_for_lifetime); RUN_TEST(version_cohort_does_not_repurpose_daemon_startup_lock_for_lifetime);
RUN_TEST(version_cohort_touch_reports_each_lost_file);
RUN_TEST(version_cohort_distinguishes_coordinated_daemon_without_connecting); RUN_TEST(version_cohort_distinguishes_coordinated_daemon_without_connecting);
RUN_TEST(version_cohort_transition_presence_is_authoritative_and_marker_checked); RUN_TEST(version_cohort_transition_presence_is_authoritative_and_marker_checked);
RUN_TEST(version_cohort_transition_shutdown_order_has_no_false_conflict); RUN_TEST(version_cohort_transition_shutdown_order_has_no_false_conflict);