Conversation
Reviewer's GuideThe PR redesigns daemon launch, replacement, and teardown around a shared lifecycle lock with verified process reaping, allowing root reloads to keep serving until the swap while preventing VPN/preview resource races. It also hardens root identity checks and trims the native loader through linker/compiler optimization and reduced decompression/file-sync overhead. Sequence diagram for serialized runtime core replacementsequenceDiagram
participant Launcher as RootSessionLauncher
participant Core as CoreProcess
participant Probe as RootDaemonProbe
participant Daemon as ExistingRootDaemon
participant Config as CompiledConfigPipeline
participant NewDaemon as NewCore
Launcher->>Config: compileDetailed(spec)
Launcher->>Core: startRoot(mode, finalYaml)
Core->>Core: withLifecycleLock
Core->>Probe: reap(record)
Probe->>Daemon: kill-TERM / kill-KILL
Probe-->>Core: verified process exited
Core->>NewDaemon: launchRoot(mode, config)
NewDaemon-->>Core: CoreEndpoint
Core-->>Launcher: CoreEndpoint
Launcher->>Launcher: awaitControllerReady()
alt compile or launch fails before swap
Launcher->>Probe: isTrackedRootProcessAlive()
Probe-->>Launcher: existing daemon still alive
Launcher-->>Launcher: markRuntimeRunning(servingMode)
end
Sequence diagram for VPN launch and daemon reapingsequenceDiagram
participant VPN as VpnTunTransport
participant Core as CoreProcess
participant Preview as PreviewCoreProcess
participant Root as RootDaemonProbe
participant Tunnel as Android VPN
participant Child as VPN Core
VPN->>Core: startVpn(config, stack, openTunnel)
Core->>Core: withLifecycleLock
Core->>Preview: stopActive()
Core->>Core: stop and verify existing VPN child
Core->>Root: reapRootDaemon()
Root-->>Core: verified root daemon exited
Core->>Tunnel: openTunnel(config)
Tunnel-->>Core: VpnTunnel(fd, gateway, dns)
Core->>Child: launchVpn(fd, gateway, dns, config, stack)
Child-->>Core: CoreEndpoint
Core-->>VPN: CoreEndpoint
VPN->>VPN: releaseReapedHost(vpnService, rootMode)
State diagram for runtime ownership during root replacementstateDiagram-v2
[*] --> RootServing
RootServing --> CompilingReplacement: compileDetailed(spec)
CompilingReplacement --> RootServing: compilation fails
CompilingReplacement --> ReapingOldRoot: startRoot()
ReapingOldRoot --> LaunchingNewRoot: reap(record) verified
LaunchingNewRoot --> RootServing: controller ready
LaunchingNewRoot --> Idle: launch fails after old root is gone
RootServing --> ReapingOldRoot: VPN or cross-mode launch
ReapingOldRoot --> LaunchingVpn: root daemon reaped
LaunchingVpn --> VpnServing: startVpn() succeeds
ReapingOldRoot --> RootServing: replacement fails before swap
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
YumeLira
added this pull request to stack #190
September 27, 2026 10:44
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="runtime/service/src/runtime/service/core/CoreProcess.kt" line_range="597-604" />
<code_context>
- } finally {
- dyingRootPid = null
- }
+ internal inline fun <T> withLifecycleLock(body: () -> T): T {
+ lifecycleLock.lock()
+ try {
+ return body()
+ } finally {
+ lifecycleLock.unlock()
}
}
</code_context>
<issue_to_address>
**issue (bug_risk):** The internal inline `withLifecycleLock` accesses the private `lifecycleLock` from its inlined body, which Kotlin rejects as a non-public API access for an inline function, so the service module fails to compile.
**Suggested fix:** Remove `inline`, or expose the lock through an appropriate `@PublishedApi internal` declaration.
```suggestion
internal fun <T> withLifecycleLock(body: () -> T): T {
lifecycleLock.lock()
try {
return body()
} finally {
lifecycleLock.unlock()
}
}
```
</issue_to_address>
### Comment 2
<location path="runtime/service/src/runtime/service/core/RootDaemonProbe.kt" line_range="32-35" />
<code_context>
+ * lock and the controller endpoint.
+ */
+internal object RootDaemonProbe {
+ fun trackedAlive(): Boolean {
+ val pid = RootDaemonState.load()?.pid ?: return false
+ if (pid <= 0) return false
+ return exec("kill -0 $pid")?.isSuccess == true
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Root liveness and reaping use only the recorded PID, and `reap` does not compare the recorded start time before signaling a matching executable. When the daemon PID is recycled, the code treats an unrelated process as the daemon and can terminate it, or reports a stale daemon as still serving.
**Triggers:** When the persisted root PID has been reused after the original daemon exits.
**Suggested fix:** Require executable and start-time identity validation in `trackedAlive` and before `reap` sends signals; discard a record on a definite identity mismatch.
</issue_to_address>
### Comment 3
<location path="runtime/service/src/runtime/service/core/PreviewCoreProcess.kt" line_range="90" />
<code_context>
controller = null
if (previous != null) {
- runCatching { previous.terminate() }
+ reap(previous)
}
}
</code_context>
<issue_to_address>
**issue (bug_risk):** `reap` ignores the result of both exit waits, so `stopLocked` returns even when the preview process remains alive after SIGKILL. The subsequent real-core launch then proceeds with the old preview process still running and sharing the runtime home/socket resources.
**Triggers:** When a preview process does not exit within the 300 ms grace period plus the 200 ms kill wait.
**Suggested fix:** Return the reap result and abort the launch, or continue waiting/fail explicitly when the process remains alive.
```suggestion
reap(previous)
check(!File("/proc/${previous.pid}").exists()) {
"preview process remains alive after reap"
}
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 3 findings to address first, and a failure in the new locking, PID identity checks, or reap paths could leave two privileged cores competing for the tunnel, routes, or controller socket, or terminate the wrong process and cause an outage. Reverting prevents future launches from using this lifecycle, but it cannot undo a networking outage or other teardown side effects that already occurred.
Blocking findings: runtime/service/src/runtime/service/core/CoreProcess.kt:604, runtime/service/src/runtime/service/core/RootDaemonProbe.kt:35, runtime/service/src/runtime/service/core/PreviewCoreProcess.kt:90
Comment on lines
+597
to
604
| internal inline fun <T> withLifecycleLock(body: () -> T): T { | ||
| lifecycleLock.lock() | ||
| try { | ||
| return body() | ||
| } finally { | ||
| lifecycleLock.unlock() | ||
| } | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): The internal inline withLifecycleLock accesses the private lifecycleLock from its inlined body, which Kotlin rejects as a non-public API access for an inline function, so the service module fails to compile.
Suggested fix: Remove inline, or expose the lock through an appropriate @PublishedApi internal declaration.
Suggested change
| internal inline fun <T> withLifecycleLock(body: () -> T): T { | |
| lifecycleLock.lock() | |
| try { | |
| return body() | |
| } finally { | |
| lifecycleLock.unlock() | |
| } | |
| } | |
| internal fun <T> withLifecycleLock(body: () -> T): T { | |
| lifecycleLock.lock() | |
| try { | |
| return body() | |
| } finally { | |
| lifecycleLock.unlock() | |
| } | |
| } |
Comment on lines
+32
to
+35
| fun trackedAlive(): Boolean { | ||
| val pid = RootDaemonState.load()?.pid ?: return false | ||
| if (pid <= 0) return false | ||
| return exec("kill -0 $pid")?.isSuccess == true |
There was a problem hiding this comment.
issue (bug_risk): Root liveness and reaping use only the recorded PID, and reap does not compare the recorded start time before signaling a matching executable. When the daemon PID is recycled, the code treats an unrelated process as the daemon and can terminate it, or reports a stale daemon as still serving.
Triggers: When the persisted root PID has been reused after the original daemon exits.
Suggested fix: Require executable and start-time identity validation in trackedAlive and before reap sends signals; discard a record on a definite identity mismatch.
| controller = null | ||
| if (previous != null) { | ||
| runCatching { previous.terminate() } | ||
| reap(previous) |
There was a problem hiding this comment.
issue (bug_risk): reap ignores the result of both exit waits, so stopLocked returns even when the preview process remains alive after SIGKILL. The subsequent real-core launch then proceeds with the old preview process still running and sharing the runtime home/socket resources.
Triggers: When a preview process does not exit within the 300 ms grace period plus the 200 ms kill wait.
Suggested fix: Return the reap result and abort the launch, or continue waiting/fail explicitly when the process remains alive.
Suggested change
| reap(previous) | |
| reap(previous) | |
| check(!File("/proc/${previous.pid}").exists()) { | |
| "preview process remains alive after reap" | |
| } |
YumeYuka
force-pushed
the
fix-daemon-lifecycle-launch-reap
branch
from
September 27, 2026 12:09
ce11f4e to
e99c1f6
Compare
YumeYuka
approved these changes
Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Ensure runtime core replacement, failure recovery, and daemon shutdown are serialized so only one active core owns the tunnel and runtime resources.
Bug Fixes:
Enhancements:
Build: