Skip to content

Commit e609b19

Browse files
author
nate
committed
fix: serialize lifetime reservation probe with acquire
1 parent 269ed7c commit e609b19

3 files changed

Lines changed: 210 additions & 15 deletions

File tree

‎src/daemon/ipc.c‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,8 @@ static char *string_format(const char *format, ...) {
231231
#ifndef _WIN32
232232
static cbm_daemon_ipc_posix_publication_hook_fn g_posix_publication_hook_for_test;
233233
static void *g_posix_publication_hook_context_for_test;
234+
static cbm_daemon_ipc_posix_lifetime_probe_hook_fn g_posix_lifetime_probe_hook_for_test;
235+
static void *g_posix_lifetime_probe_hook_context_for_test;
234236
#endif
235237
static atomic_uint g_windows_legacy_guard_release_failures_for_test;
236238

@@ -245,6 +247,17 @@ void cbm_daemon_ipc_posix_publication_hook_set_for_test(
245247
#endif
246248
}
247249

250+
void cbm_daemon_ipc_posix_lifetime_probe_hook_set_for_test(
251+
cbm_daemon_ipc_posix_lifetime_probe_hook_fn hook, void *context) {
252+
#ifndef _WIN32
253+
g_posix_lifetime_probe_hook_context_for_test = context;
254+
g_posix_lifetime_probe_hook_for_test = hook;
255+
#else
256+
(void)hook;
257+
(void)context;
258+
#endif
259+
}
260+
248261
void cbm_daemon_ipc_windows_legacy_guard_release_failures_set_for_test(unsigned int count) {
249262
atomic_store_explicit(&g_windows_legacy_guard_release_failures_for_test, count,
250263
memory_order_release);
@@ -1301,6 +1314,10 @@ static int posix_lifetime_lock_probe(const cbm_daemon_ipc_endpoint_t *endpoint,
13011314
return endpoint_runtime_still_valid(endpoint) ? 1 : -1;
13021315
}
13031316

1317+
if (g_posix_lifetime_probe_hook_for_test) {
1318+
g_posix_lifetime_probe_hook_for_test(g_posix_lifetime_probe_hook_context_for_test);
1319+
}
1320+
13041321
int fd = openat(endpoint->dir_fd, lock_name, O_RDWR | O_CLOEXEC | O_NOFOLLOW);
13051322
if (fd < 0) {
13061323
return errno == ENOENT && process_claimed == 0 ? 0 : -1;
@@ -1400,10 +1417,9 @@ static void posix_lifetime_lock_release(int fd, process_lock_entry_t *process_en
14001417
process_lock_unclaim(process_entry);
14011418
}
14021419

1403-
/* Serialize lifetime probe, claim and release within this process. Without this
1404-
* gate, a probe can observe an empty registry, then open/close the inode
1405-
* after another thread acquires its process-scoped fcntl lock. */
1406-
static pthread_mutex_t lifetime_lock_operations_mutex = PTHREAD_MUTEX_INITIALIZER;
1420+
/* Serialize lifetime operations with fork and each other. The shared guard's
1421+
* atfork handlers prevent a child from inheriting it locked by a vanished
1422+
* thread. Lifetime operations do not call private-file-lock APIs while held. */
14071423

14081424
int cbm_daemon_ipc_lifetime_reservation_try_acquire(
14091425
const cbm_daemon_ipc_endpoint_t *endpoint,
@@ -1414,24 +1430,24 @@ int cbm_daemon_ipc_lifetime_reservation_try_acquire(
14141430
if (!endpoint || !reservation_out) {
14151431
return -1;
14161432
}
1417-
if (pthread_mutex_lock(&lifetime_lock_operations_mutex) != 0) {
1433+
if (!cbm_private_file_lock_fork_guard_enter()) {
14181434
return -1;
14191435
}
14201436
int fd = -1;
14211437
process_lock_entry_t *process_entry = NULL;
14221438
int result = posix_lifetime_lock_try_acquire(endpoint, endpoint->lifetime_lock_name, &fd,
14231439
&process_entry);
14241440
if (result != 1) {
1425-
(void)pthread_mutex_unlock(&lifetime_lock_operations_mutex);
1441+
cbm_private_file_lock_fork_guard_leave();
14261442
return result;
14271443
}
14281444
cbm_daemon_ipc_lifetime_reservation_t *reservation = malloc(sizeof(*reservation));
14291445
if (!reservation) {
14301446
posix_lifetime_lock_release(fd, process_entry);
1431-
(void)pthread_mutex_unlock(&lifetime_lock_operations_mutex);
1447+
cbm_private_file_lock_fork_guard_leave();
14321448
return -1;
14331449
}
1434-
(void)pthread_mutex_unlock(&lifetime_lock_operations_mutex);
1450+
cbm_private_file_lock_fork_guard_leave();
14351451
reservation->fd = fd;
14361452
reservation->process_entry = process_entry;
14371453
reservation->owner_pid = getpid();
@@ -1444,11 +1460,11 @@ void cbm_daemon_ipc_lifetime_reservation_release(
14441460
if (!reservation) {
14451461
return;
14461462
}
1447-
if (pthread_mutex_lock(&lifetime_lock_operations_mutex) != 0) {
1448-
ipc_coordination_cleanup_fail_stop("lifetime_lock_mutex");
1463+
if (!cbm_private_file_lock_fork_guard_enter()) {
1464+
ipc_coordination_cleanup_fail_stop("lifetime_lock_fork_guard");
14491465
}
14501466
posix_lifetime_lock_release(reservation->fd, reservation->process_entry);
1451-
(void)pthread_mutex_unlock(&lifetime_lock_operations_mutex);
1467+
cbm_private_file_lock_fork_guard_leave();
14521468
free(reservation);
14531469
}
14541470

@@ -1474,11 +1490,11 @@ static bool lifetime_reservation_matches_endpoint(
14741490
}
14751491

14761492
int cbm_daemon_ipc_lifetime_reservation_probe(const cbm_daemon_ipc_endpoint_t *endpoint) {
1477-
if (!endpoint || pthread_mutex_lock(&lifetime_lock_operations_mutex) != 0) {
1493+
if (!endpoint || !cbm_private_file_lock_fork_guard_enter()) {
14781494
return -1;
14791495
}
14801496
int result = posix_lifetime_lock_probe(endpoint, endpoint->lifetime_lock_name);
1481-
(void)pthread_mutex_unlock(&lifetime_lock_operations_mutex);
1497+
cbm_private_file_lock_fork_guard_leave();
14821498
return result;
14831499
}
14841500

‎src/daemon/ipc_internal.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ typedef void (*cbm_daemon_ipc_posix_publication_hook_fn)(
110110

111111
void cbm_daemon_ipc_posix_publication_hook_set_for_test(
112112
cbm_daemon_ipc_posix_publication_hook_fn hook, void *context);
113+
typedef void (*cbm_daemon_ipc_posix_lifetime_probe_hook_fn)(void *context);
114+
void cbm_daemon_ipc_posix_lifetime_probe_hook_set_for_test(
115+
cbm_daemon_ipc_posix_lifetime_probe_hook_fn hook, void *context);
113116
void cbm_daemon_ipc_windows_legacy_guard_release_failures_set_for_test(unsigned int count);
114117
/* #1828: while non-zero, every POSIX socket-record publication write fails
115118
* with this errno. This is the deterministic stand-in for a full runtime

‎tests/test_daemon_ipc.c‎

Lines changed: 178 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ extern const char *cbm_test_program_path;
5353
#include <sys/wait.h>
5454
#include <pwd.h>
5555
#include <pthread.h>
56+
#include <time.h>
5657
#include <unistd.h>
5758
#ifdef __linux__
5859
#include <sched.h> /* #1830 userns real smoke: unshare(CLONE_NEWUSER). */
@@ -3507,8 +3508,8 @@ TEST(daemon_ipc_posix_lifetime_reservation_concurrent_queries_preserve_kernel_lo
35073508
if (reservation) {
35083509
probe_worker.endpoint = endpoint;
35093510
acquire_worker.endpoint = endpoint;
3510-
probe_started = pthread_create(&probe_thread, NULL, ipc_lifetime_concurrent_worker,
3511-
&probe_worker) == 0;
3511+
probe_started =
3512+
pthread_create(&probe_thread, NULL, ipc_lifetime_concurrent_worker, &probe_worker) == 0;
35123513
acquire_started = pthread_create(&acquire_thread, NULL, ipc_lifetime_concurrent_worker,
35133514
&acquire_worker) == 0;
35143515
}
@@ -3567,6 +3568,180 @@ TEST(daemon_ipc_posix_lifetime_reservation_concurrent_queries_preserve_kernel_lo
35673568
PASS();
35683569
}
35693570

3571+
typedef struct {
3572+
const cbm_daemon_ipc_endpoint_t *endpoint;
3573+
pthread_mutex_t mutex;
3574+
pthread_cond_t condition;
3575+
bool probe_parked;
3576+
bool release_probe;
3577+
atomic_bool acquire_started;
3578+
atomic_bool acquire_done;
3579+
cbm_daemon_ipc_lifetime_reservation_t *reservation;
3580+
int acquire_result;
3581+
int probe_result;
3582+
} ipc_lifetime_interleave_t;
3583+
3584+
static void ipc_test_pause_lifetime_probe(void *opaque) {
3585+
ipc_lifetime_interleave_t *state = opaque;
3586+
(void)pthread_mutex_lock(&state->mutex);
3587+
state->probe_parked = true;
3588+
(void)pthread_cond_broadcast(&state->condition);
3589+
while (!state->release_probe) {
3590+
(void)pthread_cond_wait(&state->condition, &state->mutex);
3591+
}
3592+
(void)pthread_mutex_unlock(&state->mutex);
3593+
}
3594+
3595+
static void *ipc_test_lifetime_probe_interleave(void *opaque) {
3596+
ipc_lifetime_interleave_t *state = opaque;
3597+
state->probe_result = cbm_daemon_ipc_lifetime_reservation_probe(state->endpoint);
3598+
return NULL;
3599+
}
3600+
3601+
static void *ipc_test_lifetime_acquire_interleave(void *opaque) {
3602+
ipc_lifetime_interleave_t *state = opaque;
3603+
atomic_store_explicit(&state->acquire_started, true, memory_order_release);
3604+
state->acquire_result =
3605+
cbm_daemon_ipc_lifetime_reservation_try_acquire(state->endpoint, &state->reservation);
3606+
atomic_store_explicit(&state->acquire_done, true, memory_order_release);
3607+
(void)pthread_mutex_lock(&state->mutex);
3608+
(void)pthread_cond_broadcast(&state->condition);
3609+
(void)pthread_mutex_unlock(&state->mutex);
3610+
return NULL;
3611+
}
3612+
3613+
TEST(daemon_ipc_posix_first_lifetime_acquire_serializes_with_probe) {
3614+
static const char key[] = "a1a1b2b2c3c3d4d4";
3615+
char parent[TEST_PATH_CAP] = {0}, runtime_dir[TEST_PATH_CAP] = {0};
3616+
char lock_path[TEST_PATH_CAP] = {0};
3617+
cbm_daemon_ipc_endpoint_t *endpoint = NULL;
3618+
ipc_lifetime_interleave_t state = {0};
3619+
pthread_t probe_thread, acquire_thread;
3620+
bool mutex_ready = false, condition_ready = false;
3621+
bool probe_started = false, acquire_started = false;
3622+
bool probe_joined = false, acquire_joined = false, acquire_blocked = false;
3623+
pid_t child = -1;
3624+
int child_status = -1, result_pipe[2] = {-1, -1};
3625+
uint8_t child_result = IPC_TEST_LOCK_ERROR;
3626+
int free_after_release = -1;
3627+
3628+
if (ipc_test_parent_new(parent, "first-lifetime-interleave")) {
3629+
endpoint = cbm_daemon_ipc_endpoint_new(key, parent);
3630+
}
3631+
if (endpoint) {
3632+
ipc_test_copy_path(runtime_dir, cbm_daemon_ipc_endpoint_runtime_dir(endpoint));
3633+
state.endpoint = endpoint;
3634+
}
3635+
if (endpoint && ipc_test_lifetime_lock_path(lock_path, runtime_dir, key) &&
3636+
pthread_mutex_init(&state.mutex, NULL) == 0) {
3637+
mutex_ready = true;
3638+
if (pthread_cond_init(&state.condition, NULL) == 0) {
3639+
condition_ready = true;
3640+
atomic_init(&state.acquire_started, false);
3641+
atomic_init(&state.acquire_done, false);
3642+
cbm_daemon_ipc_posix_lifetime_probe_hook_set_for_test(ipc_test_pause_lifetime_probe,
3643+
&state);
3644+
probe_started = pthread_create(&probe_thread, NULL, ipc_test_lifetime_probe_interleave,
3645+
&state) == 0;
3646+
if (probe_started) {
3647+
(void)pthread_mutex_lock(&state.mutex);
3648+
while (!state.probe_parked) {
3649+
(void)pthread_cond_wait(&state.condition, &state.mutex);
3650+
}
3651+
(void)pthread_mutex_unlock(&state.mutex);
3652+
acquire_started = pthread_create(&acquire_thread, NULL,
3653+
ipc_test_lifetime_acquire_interleave, &state) == 0;
3654+
if (acquire_started) {
3655+
while (!atomic_load_explicit(&state.acquire_started, memory_order_acquire)) {}
3656+
struct timespec deadline;
3657+
(void)clock_gettime(CLOCK_REALTIME, &deadline);
3658+
deadline.tv_nsec += 250000000L;
3659+
if (deadline.tv_nsec >= 1000000000L) {
3660+
deadline.tv_sec++;
3661+
deadline.tv_nsec -= 1000000000L;
3662+
}
3663+
(void)pthread_mutex_lock(&state.mutex);
3664+
int wait_status = 0;
3665+
while (!atomic_load_explicit(&state.acquire_done, memory_order_acquire) &&
3666+
wait_status == 0) {
3667+
wait_status =
3668+
pthread_cond_timedwait(&state.condition, &state.mutex, &deadline);
3669+
}
3670+
acquire_blocked =
3671+
!atomic_load_explicit(&state.acquire_done, memory_order_acquire);
3672+
state.release_probe = true;
3673+
(void)pthread_cond_broadcast(&state.condition);
3674+
(void)pthread_mutex_unlock(&state.mutex);
3675+
} else {
3676+
(void)pthread_mutex_lock(&state.mutex);
3677+
state.release_probe = true;
3678+
(void)pthread_cond_broadcast(&state.condition);
3679+
(void)pthread_mutex_unlock(&state.mutex);
3680+
}
3681+
}
3682+
if (probe_started) {
3683+
probe_joined = pthread_join(probe_thread, NULL) == 0;
3684+
}
3685+
if (acquire_started) {
3686+
acquire_joined = pthread_join(acquire_thread, NULL) == 0;
3687+
}
3688+
cbm_daemon_ipc_posix_lifetime_probe_hook_set_for_test(NULL, NULL);
3689+
}
3690+
}
3691+
if (state.reservation && pipe(result_pipe) == 0) {
3692+
child = fork();
3693+
}
3694+
if (child == 0) {
3695+
(void)close(result_pipe[0]);
3696+
child_result = ipc_test_lock_child_verdict(lock_path);
3697+
bool sent = ipc_test_fd_write_all(result_pipe[1], &child_result, sizeof(child_result));
3698+
(void)close(result_pipe[1]);
3699+
_exit(sent ? 0 : 1);
3700+
}
3701+
if (child > 0) {
3702+
(void)close(result_pipe[1]);
3703+
result_pipe[1] = -1;
3704+
bool received = ipc_test_fd_read_all(result_pipe[0], &child_result, sizeof(child_result));
3705+
(void)close(result_pipe[0]);
3706+
result_pipe[0] = -1;
3707+
if (!received) {
3708+
child_result = IPC_TEST_LOCK_ERROR;
3709+
}
3710+
while (waitpid(child, &child_status, 0) < 0 && errno == EINTR) {}
3711+
}
3712+
cbm_daemon_ipc_lifetime_reservation_release(state.reservation);
3713+
if (endpoint) {
3714+
free_after_release = cbm_daemon_ipc_lifetime_reservation_probe(endpoint);
3715+
}
3716+
for (size_t index = 0; index < 2; index++) {
3717+
if (result_pipe[index] >= 0) {
3718+
(void)close(result_pipe[index]);
3719+
}
3720+
}
3721+
if (condition_ready) {
3722+
(void)pthread_cond_destroy(&state.condition);
3723+
}
3724+
if (mutex_ready) {
3725+
(void)pthread_mutex_destroy(&state.mutex);
3726+
}
3727+
cbm_daemon_ipc_endpoint_free(endpoint);
3728+
ipc_test_remove_tree(runtime_dir, parent);
3729+
3730+
ASSERT_TRUE(probe_started);
3731+
ASSERT_TRUE(acquire_started);
3732+
ASSERT_TRUE(probe_joined);
3733+
ASSERT_TRUE(acquire_joined);
3734+
ASSERT_EQ(state.probe_result, 0);
3735+
ASSERT_EQ(state.acquire_result, 1);
3736+
ASSERT_GT(child, 0);
3737+
ASSERT_TRUE(WIFEXITED(child_status));
3738+
ASSERT_EQ(WEXITSTATUS(child_status), 0);
3739+
ASSERT_EQ(child_result, IPC_TEST_LOCK_HELD);
3740+
ASSERT_TRUE(acquire_blocked);
3741+
ASSERT_EQ(free_after_release, 0);
3742+
PASS();
3743+
}
3744+
35703745
TEST(daemon_ipc_posix_lifetime_reservation_probe_does_not_drop_held_lock) {
35713746
static const char key[] = "c1c1d2d2e3e3f4f4";
35723747
char parent[TEST_PATH_CAP] = {0};
@@ -6163,6 +6338,7 @@ SUITE(daemon_ipc) {
61636338
RUN_TEST(daemon_ipc_posix_lifetime_reservation_rejects_fork_inheritance);
61646339
RUN_TEST(daemon_ipc_posix_lifetime_reservation_probe_does_not_drop_held_lock);
61656340
RUN_TEST(daemon_ipc_posix_lifetime_reservation_concurrent_queries_preserve_kernel_lock);
6341+
RUN_TEST(daemon_ipc_posix_first_lifetime_acquire_serializes_with_probe);
61666342
RUN_TEST(daemon_ipc_posix_lifetime_reservation_try_acquire_again_does_not_drop_held_lock);
61676343
RUN_TEST(daemon_ipc_posix_child_participant_handoff_retains_legacy_bridge);
61686344
RUN_TEST(daemon_ipc_posix_publication_boundaries_recover_from_crash);

0 commit comments

Comments
 (0)