Don't wait forever for a control command reply

[sithglan]

Fixes #723.

Problem

dhcpcd-10.5 made the manager reply to a control command with an int error, and dhcpcd_readerror() consumes it with a blocking read(3):

len = read(ctx->control_fd, &error, sizeof(error));

Managers older than 10.5 have no dhcpcd_readerror() and never send that reply — the 10.3.2 client simply sent its command and exited. So a 10.5 client talking to a pre-10.5 manager blocks in read(3) forever. The manager receives and fully actions the command; only the reply the newer client waits for is missing.

This happens whenever the dhcpcd binaries are upgraded while the manager keeps running, which is the normal case for distributions that run dhcpcd -q -b from a service unit and do not restart it on upgrade. Every control command then hangs until the manager is restarted or the machine reboots.

On Debian it surfaces through ifupdown: networking.service's ExecStop runs ifdown -a, which runs dhcpcd -k <iface>, which never returns. That adds ~3 minutes to the first shutdown after the upgrade — and because the blocked read(3) is restarted after SIGTERM, systemd's 90 s stop timeout does not end it either; it takes the second 90 s timeout and a SIGKILL.

Fix

Poll the control socket with a 5 second timeout before reading, which is what dhcpcd_readdump() immediately below already does for lease dumps. On timeout, warn and assume the command was actioned — that is exactly what an older manager did. EOF is likewise treated as success, for a manager that closes without replying.

When the manager does reply, behaviour is completely unchanged: poll() returns immediately and the existing read() path runs as before.

Testing

Built and tested on Debian forky/sid, amd64, privsep enabled, interface managed by ifupdown (iface enp1s0 inet dhcp). The same 10.5.2 tree built twice, with and without this commit, run back to back against a live manager:

manager	client	elapsed	exit	result
10.3.2	10.5.2 unpatched	20 s	137	hung; had to be SIGKILLed
10.3.2	10.5.2 patched	5 s	0	warns, then proceeds
10.5.2	10.5.2 patched	8 ms	0	unchanged fast path, no warning

Unpatched, against a 10.3.2 manager:

elapsed=20s exit=137
output: sending commands to dhcpcd process

(exit 137 = SIGKILL; timeout -s TERM was not enough to end it, matching the systemd behaviour described above.)

Patched, against the same 10.3.2 manager:

elapsed=5s exit=0
output: sending commands to dhcpcd process
        timed out waiting for a reply from dhcpcd; assuming the command
        was actioned (is the running dhcpcd older than 10.5.2?)

Patched, against a matching 10.5.2 manager:

elapsed=8ms exit=0
output: sending commands to dhcpcd process

Error reporting is unaffected — a command refused by the control socket still fails promptly (main: control_open: Permission denied, exit 1, 12 ms) rather than being swallowed.

Note on the timeout semantics

I chose "warn and treat as actioned" on timeout because that matches what a pre-10.5 manager actually does — it carries out the command and never replies — so failing would make ifdown report a failure for work that was in fact done. If you would rather have a timeout be a hard error, or prefer a different duration than the 5 s borrowed from dhcpcd_readdump(), I am happy to change it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 585a3fd5-c2a8-4742-96c9-05ba17d99ca4

📥 Commits

Reviewing files that changed from the base of the PR and between 3d826e2 and d8c5d87.

📒 Files selected for processing (1)

• src/dhcpcd.8.in

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

Walkthrough

The control socket read now uses a five-second poll timeout. Interrupted polls retry. A timeout or zero-length read returns success instead of blocking indefinitely. The manual documents behavior with older managers.

Changes

Control command handling

Layer / File(s)	Summary
Bounded control reply handling src/dhcpcd.c, src/dhcpcd.8.in	Adds polling and timeout handling to dhcpcd_readerror. Timeouts and closed sockets return 0. The manual documents the behavior and updates its date.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d8c5d

Control commands no longer hang indefinitely against older managers that execute commands without replying, while compatible-manager replies and socket errors retain their expected behavior. The documented timeout behavior is ready to merge.

Suggested reviewers: rsmarples

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing indefinite waits for control command replies.
Description check	✅ Passed	The description directly explains the hang, the compatibility cause, the timeout and EOF handling, and the test results.
Linked Issues check	✅ Passed	The implementation addresses issue #723 by adding a five-second poll timeout, handling EINTR, treating timeout and EOF as successful command completion, and preserving the existing reply and error pat…
Out of Scope Changes check	✅ Passed	The changes are within scope for issue #723. The control-socket polling logic and the related manual-page documentation directly support the fix.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ColinMcInnes]

Ah the joys of upgrading older systems. I'm ok with with this change, and I appreciate the log message in case the user wasn't aware the manager was out of sync.

I would recommend adding that "timed out" message to the manual, so it can be looked up if a user sees it.

[sithglan]

Thanks! Done in d8c5d87 — the message is now documented under NOTES in dhcpcd.8, along with the fact that restarting the running manager makes it go away.

One thing worth mentioning in case it changes your view on merging: the Debian package now restarts dhcpcd on upgrade, so the hang is already worked around there. I still think it's worth having upstream — anyone upgrading the binaries without restarting the manager hits it, on any distro — but there's no longer a fire to put out, so no urgency from my side if you'd rather not carry it.