eclipse-score / eclipse-score/lifecycle

SIGSEGV during launch manager shutdown: Probably use-after-free (SIGSEGV) on `terminator_` semaphore

Open
#397 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
6
Forks
34
Avg merge
2d 3h
Merged PRs (30d)
80

Description

### Description

NOTE: This bug has been found by AI, so at first it might be verified that the source of the problem is diagnosed correctly.

During development of sandbox_options integration test, the lcm fails one time during 100 runs with SIGSEGV during shutdown.

To reproduce use this command:
```
bazel test //tests/integration/sandbox_options:sandbox_options \
--config=x86_64-linux --verbose_failures --nocache_test_results --runs_per_test=100
```

I believe this problem is also reproducible with other integration tests. Furthermore the notes to reviewer refer to changes on branch feature/honor-deployment-config-working-dir commit 4264e80131c907458fb53c454e5287eda3e3a6cf (at the time of creation of this issue), see also pr #356.

Below there is the (lengthy) description of Claude:

## Summary

During process-group termination the launch manager daemon can crash with **SIGSEGV
(exit code 139)**. It is a destroy-while-in-use race on a POSIX semaphore
(`ProcessInfoNode::terminator_`) between the OS reaper thread and a worker thread.

The bug is **pre-existing** and independent of any test code. It became visible only
because the `sandbox_options` integration test now asserts the LCM exits cleanly
(`exit_code == 0`) after `SIGTERM`; the previous test helper ignored the exit code and
silently masked the crash.

## Severity / frequency

- Reproduced **1 in 100** runs locally (also 2/100 in an earlier rebase run).
- Occurs on the normal shutdown path (SIGTERM), after the daemon test has already
PASSED and all managed processes have terminated with status 0.
- Symptom: the daemon dies right after the last process (the state-manager,
`control_daemon`, process index 0) is reaped. Last log line before the crash:
`Queuing jobs after regular termination of process wait 0 ( control_daemon )`.
No shutdown-complete markers follow.

## Repro

```
bazel test //tests/integration/sandbox_options:sandbox_options \
--config=x86_64-linux --verbose_failures --nocache_test_results --runs_per_test=100
```

Failure manifests as:
```
E AssertionError: LCM did not exit cleanly, it died with code 139
tests/utils/testing_utils/run_until_file_deployed.py:80: AssertionError
```
(139 = 128 + SIGSEGV(11).)

## Root cause

Two daemon threads operate on the same `sem_t` (`ProcessInfoNode::terminator_`):

| Thread | Path | Semaphore op |
|--------|------|--------------|
| **Reaper** | `OsHandler::run()` (os_handler.cpp:30-35) loops on `waitpid`; on child exit → `SafeProcessMap::findTerminated()` (safe_process_map.cpp:297) → `ProcessInfoNode::terminated()` | `sem_post()` — process_info_node.cpp:295-297 |
| **Worker** | `WorkerThread` → `doWork()` → `terminateProcess()` → `handleTerminationProcess()` | `sem_trywait()` poll, then `sem_destroy()` — process_info_node.cpp:534-553 |

`handleTerminationProcess()` (process_info_node.cpp:530-554):

```cpp
static_cast(terminator_.init(0U, false)); // 534 sem_init
has_semaphore_.store(true); // 535
...
if ((requestTermination(pid_) == kFail) ||
(terminator_.timedWait(termination_timeout_ms_) == kSuccess)) { // 540-541 sem_trywait poll
LM_LOG_DEBUG() << "Queuing jobs after regular termination ..."; // 543 <-- last log seen
} else {
handleForcedTermination();
}
has_semaphore_.store(false); // 552
static_cast(terminator_.deinit()); // 553 sem_destroy
```

`terminated()` (process_info_node.cpp:295-297), on the reaper thread:

```cpp
if (has_semaphore_.exchange(false)) { // 295
static_cast(terminator_.post()); // 297 sem_post
}
```

`timedWait` is a polling loop over `sem_trywait` (semaphore.cpp:49-81), so the worker
is not blocked in the kernel — it observes the post's incremented count on its next
poll and returns immediately.

### Crashing interleaving

1. Reaper: `has_semaphore_.exchange(false)` returns `true` → enters the `if`, begins
`sem_post(&sem_)`.
2. Worker: next `sem_trywait` poll sees the count → `timedWait` returns `kSuccess` →
logs line 543 → `has_semaphore_.store(false)` → **`sem_destroy(&sem_)`** (line 553).
3. Reaper: `sem_post()` is still executing internal bookkeeping on a semaphore that was
just destroyed → **undefined behavior → SIGSEGV**.

The `has_semaphore_` atomic only prevents a *redundant* post; it does **not** serialize
an *in-flight* `sem_post()` against `sem_destroy()`. POSIX: destroying a semaphore while
another thread operates on it is undefined behavior.

It concentrates on `control_daemon` (process index 0, the last-reaped state manager)
because that is where the worker's per-node `deinit()` and the reaper's final `post()`
converge in time.

## Suggested fix

Preferred: make `terminator_`'s lifetime equal the node's lifetime — `init()` once in the
constructor, `deinit()` once in the destructor — instead of `init`/`deinit` around every
`handleTerminationProcess()`. At the start of `handleTerminationProcess()` drain any stale
count with a non-blocking `sem_trywait` loop before arming `has_semaphore_`. `sem_post` on
a live (never-destroyed) semaphore is always safe, so the destroy-during-post race
disappears.

Alternatives (clunkier): a mutex serializing `post()`/`deinit()`, or an "in-post" flag the
reaper sets and the worker spins on before calling `deinit()`.

## Note for reviewers

The `tests/utils/testing_utils/run_until_file_deployed.py` change that asserts
`exit_code == 0` after SIGTERM is what surfaced this real product crash. Reverting that
assertion would re-hide the use-after-free, not resolve it.

## Affected files

- `score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp`
(`terminated()` line 295-297; `handleTerminationProcess()` line 530-554)
- `score/launch_manager/src/daemon/src/process_group_manager/details/os_handler.cpp` (reaper loop)
- `score/launch_manager/src/daemon/src/process_group_manager/details/safe_process_map.cpp:297`
- `score/launch_manager/src/daemon/src/osal/details/posix/semaphore.cpp` (POSIX sem_* wrappers)

### Analysis results

_No response_

### Solution

_No response_

### Error Occurrence Rate

None

### How to reproduce

_No response_

### Supporting Information

_No response_

### Classification

Major

### First Affected Release

not released (main)

### Last Affected Release

not released (main)

### Expected Fixed Release

before release (main)

### Category

- [ ] Safety Relevant
- [ ] Security Relevant

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with process_info_node.cpp at terminated() and handleTerminationProcess(), then trace the reaper path through os_handler.cpp and safe_process_map.cpp. Reproduce with the provided Bazel command and inspect the POSIX semaphore wrappers in semaphore.cpp. Done means the shutdown race is addressed and sandbox_options completes repeated runs without SIGSEGV or a nonzero exit.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, linux
Domain
operating-systems, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.