Skip to content

Fix cron:run and the scheduler: non-blocking background tasks - #340

Merged
techmahedy merged 3 commits into
doppar:4.xfrom
techmahedy:techmahedy-4.x
Sep 26, 2026
Merged

techmahedy merged 3 commits into
doppar:4.xfrom
techmahedy:techmahedy-4.x

Conversation

@techmahedy

@techmahedy techmahedy commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Pull Request Checklist

Q A
Branch? 4.x
Bug fix? yes
New feature? no
Deprecations? no
Issues yes
License MIT

Description

cron:run did not behave as documented in several ways that only show up on real servers: inBackground() blocked the scheduler, noOverlap() could be defeated by the very runs that skipped a task, the documented cron:run --daemon crontab line started a new daemon every minute, and children could run under the wrong PHP or from the wrong directory. This PR fixes them, makes the scheduler work on hosts that disable process functions (common on cPanel), and adds real-process tests, which the scheduler did not have. Before this, runInBackground, runInForeground and runStandardMode had no test coverage at all.

Each problem below was reproduced with the real command before it was fixed, and every fix has a test that fails when the fix is removed.

Problems found and fixed

# Problem Evidence Fix
1 inBackground() made cron:run wait for the whole job 20 s job: cron:run took 20.2 s. The background subshell inherited the pipe shell_exec() reads. Redirect the whole group's stdout, stderr and stdin to /dev/null. Now returns in ~0.1 s.
2 A run that skipped a locked task released the lock of the run still working Its ScheduledCommand::__destruct called cleanup() unconditionally. With 8 concurrent probes, 3-4 of them got the lock. An instance releases only a lock it acquired (ownsLock). Background jobs are released by cron:finish.
3 Checking for a running copy and taking the lock were two separate steps Two runs starting together could both pass. The check-and-lock now runs under an exclusive flock on a .guard file.
4 cron:run --daemon started a second daemon; the docs said it is ignored Two daemons running, PID file overwritten. The daemon holds a non-blocking flock on storage/schedule/cron_daemon.lock for its lifetime. It cannot go stale (the OS releases it on kill -9) and two simultaneous starts cannot both win.
5 Foreground tasks ran whichever php was first on PATH A decoy php on PATH was executed. Cron has a minimal PATH and cPanel keeps several PHP versions. Children use SchedulePool::phpBinary(), the binary running the scheduler (falls back if PHP_BINARY is FPM/CGI/LiteSpeed).
6 Background tasks used a relative pool and no working directory Started from / (cPanel starts jobs in the home directory): Could not open input file: pool, while cron:run still reported success. Absolute PHP and pool paths, and cd into the application root.
7 Quoted arguments were split on whitespace --id="hello world" reached the command as --id="hello and world". SchedulePool::splitCommand() tokenizes like a shell. Unbalanced quotes fall back to the old whitespace split, so nothing that worked before breaks.
8 A disabled shell_exec was a fatal Error that killed the whole run Reproduced with disable_functions. Background start tries shell_exec, then exec, then proc_open. Foreground uses proc_open, then exec, then runs in the scheduler process. If nothing works, a clear error is reported.
9 Lock and last-run files lived in the shared system temp dir, named only by a hash of the command Two applications with the same scheduled command shared one lock. Predictable names in a shared /tmp. Moved to the application's own storage/schedule.
10 posix_kill() threw a ValueError for a garbage PID in a lock file Found by a test. PID range guard plus a catch.
11 cron:run always exited 0 Exits non-zero when a task could not be started or exited with an error.
12 One task failing to start aborted the rest catch (\Exception) did not catch Error. Catches \Throwable, records the failure, continues with the next task.

Other changes

  • Process helpers in SchedulePool: phpBinary(), poolScript(), buildProcessArguments(), splitCommand(), isFunctionEnabled(), startDetached(), and a portable isProcessRunning() (posix_kill, then /proc, then ps). The old version returned false on macOS (no /proc) and depended on shell_exec.
  • cron:finish looks for lock files in storage/schedule, and still in the system temp directory so a job started before an upgrade is found. It tolerates a half-written PID file.
  • cron:daemon uses the shared process check.
  • Missing schedule class: if App\Schedule\Schedule does not exist, cron:run says so and exits non-zero instead of a fatal error.
  • Windows: background execution is not available. inBackground() tasks run in the foreground with a warning, instead of running invalid shell syntax.
  • Side effect fixed: the old destructor called cleanup(), which also deleted the throttle file of noOverlap() tasks and the last-run file of second-based tasks on every run. It now releases only the lock.
  • Testable seams in CronRunCommand: makeSchedule(), startDetached(), buildBackgroundCommand(), runToCompletion().

Behavior changes to be aware of

  • cron:run exits with a non-zero status when a task fails. Crontab entries that redirect output are unaffected, but monitors that check the exit status will now see failures.
  • Lock files moved from the system temp directory to storage/schedule. cron:finish still finds old ones. An old temp-dir lock is not seen by the new code, so a job that was already running during the upgrade may start once more.
  • inBackground() tasks now return immediately. Code that accidentally relied on cron:run staying alive while a background job ran no longer gets that.
  • A task with noOverlap() that is skipped no longer clears the lock of the running copy. This is what makes overlap protection actually work.

Checklist

  • Tests have been added or updated
  • Documentation has been updated if necessary
  • Code follows the project coding standards
  • All tests pass locally

@techmahedy techmahedy added bug Something isn't working enhancement New feature or request labels Sep 26, 2026
@techmahedy
techmahedy merged commit 424a53d into doppar:4.x Sep 26, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant