[9.x] Refactor scheduled event lifecycle (#39539)

* Scheduled event lifecycle refactor

* Address background callbacks

* Code style

* StyleCI

* formatting

Co-authored-by: Taylor Otwell <taylor@laravel.com>

src/Illuminate/Console/Scheduling/CallbackEvent.php

@@ -6,6 +6,7 @@
 use Illuminate\Support\Reflector;
 use InvalidArgumentException;
 use LogicException;
+use RuntimeException;
 use Throwable;
 
 class CallbackEvent extends Event
@@ -24,11 +25,25 @@ class CallbackEvent extends Event
      */
     protected $parameters;
 
+    /**
+     * The result of the callback's execution.
+     *
+     * @var mixed
+     */
+    protected $result;
+
+    /**
+     * The exception that was thrown when calling the callback, if any.
+     *
+     * @var \Throwable|null
+     */
+    protected $exception;
+
     /**
      * Create a new event instance.
      *
      * @param  \Illuminate\Console\Scheduling\EventMutex  $mutex
-     * @param  string  $callback
+     * @param  string|callable  $callback
      * @param  array  $parameters
      * @param  \DateTimeZone|string|null  $timezone
      * @return void
@@ -50,58 +65,64 @@ public function __construct(EventMutex $mutex, $callback, array $parameters = []
     }
 
     /**
-     * Run the given event.
+     * Run the callback event.
      *
      * @param  \Illuminate\Contracts\Container\Container  $container
      * @return mixed
      *
-     * @throws \Exception
+     * @throws \Throwable
      */
     public function run(Container $container)
     {
-        if ($this->description && $this->withoutOverlapping &&
-            ! $this->mutex->create($this)) {
-            return;
-        }
-
-        $pid = getmypid();
-
-        register_shutdown_function(function () use ($pid) {
-            if ($pid === getmypid()) {
-                $this->removeMutex();
-            }
-        });
-
-        parent::callBeforeCallbacks($container);
-
-        try {
-            $response = is_object($this->callback)
-                        ? $container->call([$this->callback, '__invoke'], $this->parameters)
-                        : $container->call($this->callback, $this->parameters);
-
-            $this->exitCode = $response === false ? 1 : 0;
-        } catch (Throwable $e) {
-            $this->exitCode = 1;
+        parent::run($container);
 
-            throw $e;
-        } finally {
-            $this->removeMutex();
-
-            parent::callAfterCallbacks($container);
+        if ($this->exception) {
+            throw $this->exception;
         }
 
-        return $response;
+        return $this->result;
     }
 
     /**
-     * Clear the mutex for the event.
+     * Determine if the event should skip because another process is overlapping.
+     *
+     * @return bool
+     */
+    public function shouldSkipDueToOverlapping()
+    {
+        return $this->description && parent::shouldSkipDueToOverlapping();
+    }
+
+    /**
+     * Indicate that the callback should run in the background.
      *
      * @return void
+     *
+     * @throws \RuntimeException
      */
-    protected function removeMutex()
+    public function runInBackground()
+    {
+        throw new RuntimeException('Scheduled closures can not be run in the background.');
+    }
+
+    /**
+     * Run the callback.
+     *
+     * @param  \Illuminate\Contracts\Container\Container  $container
+     * @return int
+     */
+    protected function execute($container)
     {
-        if ($this->description && $this->withoutOverlapping) {
-            $this->mutex->forget($this);
+        try {
+            $this->result = is_object($this->callback)
+                ? $container->call([$this->callback, '__invoke'], $this->parameters)
+                : $container->call($this->callback, $this->parameters);
+
+            return $this->result === false ? 1 : 0;
+        } catch (Throwable $e) {
+            $this->exception = $e;
+
+            return 1;
         }
     }
 
@@ -121,13 +142,7 @@ public function withoutOverlapping($expiresAt = 1440)
             );
         }
 
-        $this->withoutOverlapping = true;
-
-        $this->expiresAt = $expiresAt;
-
-        return $this->skip(function () {
-            return $this->mutex->exists($this);
-        });
+        return parent::withoutOverlapping($expiresAt);
     }
 
     /**
@@ -145,9 +160,21 @@ public function onOneServer()
             );
         }
 
-        $this->onOneServer = true;
+        return parent::onOneServer();
+    }
 
-        return $this;
+    /**
+     * Get the summary of the event for display.
+     *
+     * @return string
+     */
+    public function getSummaryForDisplay()
+    {
+        if (is_string($this->description)) {
+            return $this->description;
+        }
+
+        return is_string($this->callback) ? $this->callback : 'Callback';
     }
 
     /**
@@ -161,16 +188,14 @@ public function mutexName()
     }
 
     /**
-     * Get the summary of the event for display.
+     * Clear the mutex for the event.
      *
-     * @return string
+     * @return void
      */
-    public function getSummaryForDisplay()
+    protected function removeMutex()
     {
-        if (is_string($this->description)) {
-            return $this->description;
+        if ($this->description) {
+            parent::removeMutex();
         }
-
-        return is_string($this->callback) ? $this->callback : 'Callback';
     }
 }

src/Illuminate/Console/Scheduling/Event.php

@@ -188,66 +188,81 @@ public function getDefaultOutput()
      *
      * @param  \Illuminate\Contracts\Container\Container  $container
      * @return void
+     *
+     * @throws \Throwable
      */
     public function run(Container $container)
     {
-        if ($this->withoutOverlapping &&
-            ! $this->mutex->create($this)) {
+        if ($this->shouldSkipDueToOverlapping()) {
             return;
         }
 
-        $this->runInBackground
-                    ? $this->runCommandInBackground($container)
-                    : $this->runCommandInForeground($container);
+        $exitCode = $this->start($container);
+
+        if (! $this->runInBackground) {
+            $this->finish($container, $exitCode);
+        }
     }
 
     /**
-     * Get the mutex name for the scheduled command.
+     * Determine if the event should skip because another process is overlapping.
      *
-     * @return string
+     * @return bool
      */
-    public function mutexName()
+    public function shouldSkipDueToOverlapping()
     {
-        return 'framework'.DIRECTORY_SEPARATOR.'schedule-'.sha1($this->expression.$this->command);
+        return $this->withoutOverlapping && ! $this->mutex->create($this);
     }
 
     /**
-     * Run the command in the foreground.
+     * Run the command process.
      *
      * @param  \Illuminate\Contracts\Container\Container  $container
-     * @return void
+     * @return int
+     *
+     * @throws \Throwable
      */
-    protected function runCommandInForeground(Container $container)
+    protected function start($container)
     {
         try {
             $this->callBeforeCallbacks($container);
 
-            $this->exitCode = Process::fromShellCommandline(
-                $this->buildCommand(), base_path(), null, null, null
-            )->run();
-
-            $this->callAfterCallbacks($container);
-        } finally {
+            return $this->execute($container);
+        } catch (Throwable $exception) {
             $this->removeMutex();
+
+            throw $exception;
         }
     }
 
     /**
-     * Run the command in the background.
+     * Run the command process.
+     *
+     * @param  \Illuminate\Contracts\Container\Container  $container
+     * @return int
+     */
+    protected function execute($container)
+    {
+        return Process::fromShellCommandline(
+            $this->buildCommand(), base_path(), null, null, null
+        )->run();
+    }
+
+    /**
+     * Mark the command process as finished and run callbacks/cleanup.
      *
      * @param  \Illuminate\Contracts\Container\Container  $container
+     * @param  int  $exitCode
      * @return void
      */
-    protected function runCommandInBackground(Container $container)
+    public function finish(Container $container, $exitCode)
     {
-        try {
-            $this->callBeforeCallbacks($container);
+        $this->exitCode = (int) $exitCode;
 
-            Process::fromShellCommandline($this->buildCommand(), base_path(), null, null, null)->run();
-        } catch (Throwable $exception) {
+        try {
+            $this->callAfterCallbacks($container);
+        } finally {
             $this->removeMutex();
-
-            throw $exception;
         }
     }
 
@@ -277,24 +292,6 @@ public function callAfterCallbacks(Container $container)
         }
     }
 
-    /**
-     * Call all of the "after" callbacks for the event.
-     *
-     * @param  \Illuminate\Contracts\Container\Container  $container
-     * @param  int  $exitCode
-     * @return void
-     */
-    public function callAfterCallbacksWithExitCode(Container $container, $exitCode)
-    {
-        $this->exitCode = (int) $exitCode;
-
-        try {
-            $this->callAfterCallbacks($container);
-        } finally {
-            $this->removeMutex();
-        }
-    }
-
     /**
      * Build the command string.
      *
@@ -931,6 +928,16 @@ public function preventOverlapsUsing(EventMutex $mutex)
         return $this;
     }
 
+    /**
+     * Get the mutex name for the scheduled command.
+     *
+     * @return string
+     */
+    public function mutexName()
+    {
+        return 'framework'.DIRECTORY_SEPARATOR.'schedule-'.sha1($this->expression.$this->command);
+    }
+
     /**
      * Delete the mutex for the event.
      *

src/Illuminate/Console/Scheduling/ScheduleFinishCommand.php

@@ -49,7 +49,7 @@ public function handle(Schedule $schedule)
         collect($schedule->events())->filter(function ($value) {
             return $value->mutexName() == $this->argument('id');
         })->each(function ($event) {
-            $event->callafterCallbacksWithExitCode($this->laravel, $this->argument('code'));
+            $event->finish($this->laravel, $this->argument('code'));
 
             $this->laravel->make(Dispatcher::class)->dispatch(new ScheduledBackgroundTaskFinished($event));
         });