Repository navigation
fix: parse ps output for PIDs narrower than the PID column - #406
Conversation
ps right-aligns the PID column, so any PID with fewer digits than the
column width is emitted with leading padding (" 1637 /path/to/binary").
listAllProcesses split each line at the first space, which for a padded
line is index 0, producing an empty PID field that fails to parse and
causes the line to be dropped. Only processes whose PID exactly fills
the column were ever returned -- on macOS, where the column is 5 wide,
that means only 5-digit PIDs.
Measured on a developer machine: ps reported 918 processes and
listAllProcesses returned 389, all of them 5-digit PIDs.
The practical effect is on findDeviceKitProcessForDevice, which fails
with "agent process not found" whenever the running agent's PID is
narrower than the column. StartAgent then treats the healthy agent as
absent, relaunches it on a newly allocated port, and simctl launch
returns the already-running instance still listening on its original
port. The readiness wait on the new port times out and the error path
terminates the working agent, so the agent is torn down on every call.
This is most visible on freshly booted CI machines, whose PIDs are
still low; hosts that have been up long enough to pass 10000 happen to
parse correctly.
Trim the leading padding before splitting off the PID field, and
extract the parsing into parsePsOutput so it can be tested without
shelling out.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe process listing parser was extracted into ChangesProcess listing parser
Estimated code review effort: 3 (Moderate) | ~20 minutes Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to The parser now retains right-aligned PIDs while preserving process command and environment data. The change is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@bjjeong excellent find! I'm amazed that I never ran into an issue, cause I haven't rebooted for over a month :). Will merge in a bit and credit in CHANGELOG. Thanks!! |
2 participants
Problem
listAllProcesses(devices/simulator.go) drops most of the process table.psright-aligns the PID column, so a PID with fewer digits than the column width is emitted with leading padding:The parser splits each line at the first space. For a padded line that index is
0, soline[:spaceIndex]is the empty string,strconv.Atoifails, and the line is skipped. (The existingstrings.TrimSpacedoesn't help — it runs on an already-empty slice.)The result is that only processes whose PID exactly fills the column are ever returned. On macOS the column is 5 wide, so only 5-digit PIDs survive. Measured on a developer machine:
Impact
The affected consumer is
findDeviceKitProcessForDevice, which returnsagent process not foundwhenever the running agent's PID is narrower than the column. From there:StartAgent'sgetDeviceKitPort()probe fails, so a healthy, already-running agent looks absent.FindAvailablePortInRange.simctl launchon an already-running app returns the existing instance — the newDEVICEKIT_LISTEN_PORTis never applied, and the agent keeps listening on its original port.WaitForAgentWithDiagnosticspolls the new port and times out._ = s.TerminateApp(agentBundleID)(devices/simulator.go:566), killing the working agent.So the agent is torn down on essentially every call. We hit this as a CI flake: on a fresh runner the agent was verifiably alive (PID 1637, listening on 13180, health endpoint 200, visible to
ps -E), yet the nextmobileclicall reportedagent process not foundand killed it.It reproduces deterministically as a function of PID width, which is why it looks environment-specific: long-lived hosts that have passed PID 10000 parse correctly by accident, while freshly booted CI machines still at low PIDs fail every time. Forcing the PID counter past 9999 on an affected runner — changing nothing else — made the agent get found and reused on every call.
Fix
Trim the leading padding before splitting off the PID field. The parsing is extracted into
parsePsOutputso it can be tested without shelling out;listAllProcesseskeeps its existing signature and behaviour.The header line is still skipped —
"PID COMMAND"failsAtoi— and the command field, whichextractEnvValuereadsDEVICEKIT_LISTEN_PORTback out of, is unchanged.Tests
devices/simulator_process_test.gocovers padded PIDs, header skipping, command/environment preservation, agent lookup by device path, and malformed lines. All five fail onmainand pass with the fix:Verified against the real process table: 918 of 918 processes found, up from 389.
go test ./...passes across the repo.gofmtandgo vetare clean, andgolangci-lint run ./devices/...reports the same 30 pre-existing issues insimulator.gobefore and after — no new ones, and none in the new test file.Summary by CodeRabbit