cron: own spawned process ref and temp path in RAII fields

[robobun]

Problem

• No user-visible change. Cleanup of the OS-level jobs behind Bun.cron(path, schedule, title) and Bun.cron.remove(title).
• Each job holds one ref on the helper process it spawns and a temp file it must delete on the way out. Both were plain fields: a raw pointer and a path.
• Releasing them was written out four times (each job's check_finished plus a hand-written Drop per job), so nothing stopped a new stage from dropping the pointer without detaching, or skipping the unlink.

Fix

• Two cron-private wrapper types own the obligations: one detaches and derefs the process on drop, the other unlinks the path on drop. The fields switch to them and both hand-written Drop impls go away.
• On macOS the plist is disarmed with keep where the old code took the path out, since launchctl bootstrap needs the file to stay.
• Property to check: the generated field drops run the same statements the deleted Drop bodies ran, in the same order, inside the same enter()/exit() bracket. The one new drop point, the slot assignment on spawn, always overwrites None.
• Verification: refactor only, no new test. cargo check and cargo clippy are clean; the existing cron tests pass (81 pass, 63 skip, 0 fail).

Background

• Bun.cron has an in-process callback flavour (untouched) and OS-level jobs that spawn crontab, launchctl or schtasks. Only the latter change.
• Process, bun's spawned-child handle, is intrusively refcounted. to_process returns a raw pointer carrying one ref; the holder must detach() it (exit callbacks stop reaching the job) and deref() it when done.
• The helper reads its input from a temp file (crontab text, schtasks XML, or a plist on macOS), removed when the job ends. Exception: launchctl bootstrap takes over the plist, and the remove job writes no file on macOS.
• RAII here means the cleanup lives in a wrapper's Drop, so it runs wherever the value is dropped: in check_finished, on error paths, and when the job is freed.

Original description

What

CronRegisterJob and CronRemoveJob (the Bun.cron(path, schedule, title) / Bun.cron.remove(title) OS-level jobs in src/runtime/api/cron.rs) each held the +1 ref returned by to_process as process: Option<*mut Process> and the file they have to remove when they go away as tmp_path: Option<ZString>. Releasing them was written out four times (check_finished on both jobs, plus a hand-written impl Drop on each job):

if let Some(proc) = self.process.take() {
    // SAFETY: intrusive-RC pointer; we hold a ref.
    unsafe {
        (*proc).detach();
        Process::deref(proc);
    }
}
if let Some(p) = self.tmp_path.take() {
    let _ = sys::unlink(&p);
}

Both obligations now live in two small cron-private types, and the fields become process: Option<HeldProcess> and tmp_path: Option<UnlinkOnDrop>:

struct HeldProcess(NonNull<Process>);   // Drop: detach() + Process::deref()
struct UnlinkOnDrop(ZString);           // Drop: sys::unlink(); Deref<Target = ZStr>

HeldProcess::adopt is the one place that takes over the ref from to_process (end of spawn_cmd_prepare, which still returns the raw pointer spawn_cmd_generic works with); SpawnCmdTarget::process_slot returns &mut Option<HeldProcess>; both check_finished sites become self.process = None;. The four sites that arm a file (prepare_install_crontab, prepare_filtered_crontab, prepare_plist, prepare_schtasks_create) wrap the path in UnlinkOnDrop, the File::openat calls are unchanged through Deref, and macOS prepare_bootstrap disarms with UnlinkOnDrop::keep at the point where it used to take() the path (the plist is handed to launchctl bootstrap and must stay). Both impl Drop for Cron*Job blocks are deleted; the two LIFETIMES comments describing the raw pointer go with them. CronRemoveJob never writes a file on macOS, so its tmp_path field is now #[cfg(not(target_os = "macos"))] (without the gate the field is never read there and dead_code rejects it).

The four copies of the detach/deref unsafe block are replaced by one in HeldProcess::drop, plus the new_unchecked in adopt, its call site, and the ptr::read in the macOS-only keep.

Why

Who releases the process ref and who unlinks the file is now answered by the field types, a job can no longer drop the pointer without detaching (or unlink without having armed), and a new spawn stage cannot forget either cleanup. Option<HeldProcess> uses the NonNull niche (one word, down from two) and Option<UnlinkOnDrop> keeps Option<ZString>'s layout; the generated field drops execute exactly the statements the removed Drop bodies executed, in the same order (process is declared before tmp_path), still inside the enter()/exit() bracket in finish(). The only new drop glue is on the slot assignment in spawn_cmd_prepare, whose previous value is always None because check_finished clears it before any further spawn.

Part of a series of small type-system hardening changes; each PR stands alone.

Verification

cargo check and cargo clippy are clean for the touched crates. Debug build succeeds; bun bd test test/js/bun/cron/cron.test.ts test/js/bun/cron/in-process-cron.test.ts: 81 pass, 63 skip, 0 fail (144 tests across 2 files).

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e3d3e7d0-2bc0-4994-9c5f-3a3afdfd9fd5

📥 Commits

Reviewing files that changed from the base of the PR and between 83ad764 and f14eb44.

📒 Files selected for processing (1)

• src/runtime/api/cron.rs

---

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

[robobun]

^{Updated 4:15 PM PT - Aug 11th, 2026}

✅ @robobun, your commit f14eb4419b98d406941d06b44e6733e82091d074 passed in Build #92542! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37660

That installs a local version of the PR into your bun-37660 executable, so you can run:

bun-37660 --bun

[github-actions]

This PR may be a duplicate of:

1. Deduplicate cron job state machines and DNS pending-cache drains #37363 - Also rewrites the process: Option<*mut Process> / tmp_path cleanup in src/runtime/api/cron.rs, deleting the same two hand-written impl Drop for Cron*Job blocks and collapsing the four detach/deref + unlink sites into a single Drop (via a shared CronJobCommon base struct instead of RAII field types).

🤖 Generated with Claude Code

[robobun]

Not the same change, but the overlap is real: #37363 merges the two job structs into a shared CronJobCommon and, as part of that, also collapses the hand-written Drop impls. This PR only changes the types of the two cleanup fields. The two conflict textually in cron.rs, so whichever lands second needs a rebase; if #37363 goes first I will redo this one on top of it, where it shrinks to swapping the two field types inside CronJobCommon.

[claude]

LGTM — mechanical RAII refactor, behavior-preserving.

What was reviewed:

• HeldProcess::drop and UnlinkOnDrop::drop execute the same statements as the four removed blocks; field declaration order (process before tmp_path) preserves the old manual drop order, and drop(job) in finish() still runs inside enter()/exit().
• Slot assignment in spawn_cmd_prepare — confirmed the previous value is always None (cleared by check_finished before Advance, None at construction), so no spurious detach on re-spawn.
• prepare_bootstrap's take().map(UnlinkOnDrop::keep) disarms without unlinking (ManuallyDrop + ptr::read), matching the old take() semantics; the #[cfg(not(macos))] gate on CronRemoveJob.tmp_path is correct — only prepare_filtered_crontab (already non-macOS) writes it.
• spawn_cmd_generic still receives a live *mut Process: the ref is parked in process_slot() before the raw pointer is returned.

Extended reasoning...

Overview

Single-file refactor in src/runtime/api/cron.rs introducing two cron-private RAII wrappers: HeldProcess(NonNull<Process>) (Drop = detach() + Process::deref()) and UnlinkOnDrop(ZString) (Drop = sys::unlink; Deref<Target = ZStr>; macOS-only keep() disarm). CronRegisterJob and CronRemoveJob now hold Option<HeldProcess> / Option<UnlinkOnDrop> instead of Option<*mut Process> / Option<ZString>. Four hand-written cleanup sites (two check_finished bodies + two impl Drop) collapse to field-drop glue and self.process = None. SpawnCmdTarget::process_slot return type updated at all three sites. CronRemoveJob.tmp_path gains #[cfg(not(target_os = "macos"))] since it is never written on macOS.

Security risks

None. No untrusted input handling changes; the paths passed to sys::unlink are the same ZString values as before (temp files the job itself created via make_temp_path / plist path). No new FFI surface.

Level of scrutiny

Medium — this touches intrusive refcount lifetime and unsafe blocks around Process, so I traced each obligation:

• Drop order: process is declared before tmp_path in both structs, so Rust's field-drop order matches the deleted manual Drop bodies exactly. Both run inside the ev.enter()/ev.exit() bracket via the explicit drop(job) in finish(), unchanged.
• Re-spawn drop glue: *s!().process_slot() = Some(held) would drop a prior HeldProcess if one existed. Verified via maybe_finished → check_finished (sets self.process = None before returning Advance) → advance_state → next spawn, and initial construction sets None, so the slot is always empty here.
• Raw pointer liveness in spawn_cmd_generic: held is stored in the job before held.as_ptr() is returned, so the subsequent (*process).set_exit_handler / watch_or_reap still see a ref-held Process — identical to before.
• keep(): standard ManuallyDrop + ptr::read disarm; no double-drop, no leak of the ZString.
• cfg gate: CronRemoveJob on macOS uses unlink_plist (builds path fresh) and never touches tmp_path; the only writer prepare_filtered_crontab is already #[cfg(not(target_os = "macos"))], and the constructor initializer is gated to match.

Other factors

The change directly implements REVIEW.md's "pair every acquisition with its release at the acquisition site" via RAII, reducing four copies of an unsafe detach/deref block to one. Option<HeldProcess> picks up the NonNull niche. No user-facing API or test changes; existing cron test suite passes per the PR description. Net unsafe surface shrinks. Straightforward, well-documented, and self-contained.