[11.x] delegate ProcessDriver@defer() to ProcessDriver@run() method

[rodrigopedra]

Closes #52790

Current implementation of ProcessDriver::defer() includes output redirection and background on the command array:

framework/src/Illuminate/Concurrency/ProcessDriver.php

Lines 60 to 76 in 05d9204

	public function defer(Closure|array $tasks): DeferredCallback
	{
	$php = $this->phpBinary();
	$artisan = $this->artisanBinary();
	return defer(function () use ($tasks, $php, $artisan) {
	foreach (Arr::wrap($tasks) as $task) {
	$this->processFactory->path(base_path())->env([
	'LARAVEL_INVOKABLE_CLOSURE' => serialize(new SerializableClosure($task)),
	])->run([
	$php,
	$artisan,
	'invoke-serialized-closure 2>&1 &',
	]);
	}
	});
	}

This ends building up this command line:

"/usr/bin/php" "artisan" "invoke-serialized-closure 2>&1 &"

As the third-part is quoted, when building the command, Artisan ends up looking for a command called literally invoke-serialized-closure 2>&1 &, including the 2>&1 & as the command name, which it fails to find.

This PR

• Delegates the ProcessDriver@defer() to use the ProcessDriver@run() method

• Same approach is used by the ForkDriver:

  framework/src/Illuminate/Concurrency/ForkDriver.php

  Lines 25 to 28 in 05d9204

  	public function defer(Closure|array $tasks): DeferredCallback
  	{
  	return defer(fn () => $this->run($tasks));
  	}

• I chose this approach as, from the method's docblock description, and from previous implementation (the & at the end), the deferred tasks should be run in the background and thus the implementation would be almost the same as the ProcessDriver::run() method (minus the output processing).

No tests were added, as this is not dependent on user's parameters, and I have no idea how to write a test to prevent it from happening in the future.

[taylorotwell]

This isn't totally the same. The invoke-serialized-closure 2>&1 & is what puts the process in the background.

[rodrigopedra]

I am on mobile right now, so it is hard to link to source, but on symfony/process documentation it says the Process@start() method executes the process on background.

I tried without the wait() with the issue's code, but as soon the artisan process dies it doesn't spawn the background processes without the wait() call.

[rodrigopedra]

Link to symfony's docs:

https://symfony.com/doc/current/components/process.html#running-processes-asynchronously

[rodrigopedra]

It is actually not in background, but asynchronously, that might be why it kills the child process as soon as the calling process dies.

On the issue's code this was working as expected. Although the artisan calling code waits for the child's to end.

But the main issue is that passing the command as an array it quotes everything, I even tested passing the 2>&1 & as separate array items, but then, as they get quoted, artisan thinks they are an argument and complains either that it expects a single argument (when splitting 2>&1, and &), or that it couldn't unserialize the code (when passing these as a single separate argument).

I didn't test calling the command as a string, maybe it could work. But unfortunately I am out-of-office right now.

[rodrigopedra]

One more thing, even if I the & at the end sends the process to the background, depending on how proc_open() works (symfony's docs says it uses it to run the processes) the child processes might get killed when the calling process ends.

On Linux, at least, it might be needed to pair it with a command like nohup to detach the child process from the main process, if the goal is to truly send a process to the background, and have it to be executed independently, regardless if the calling command ends.

[taylorotwell]

The current code works exactly how I expect it to on my machine. 😅

What OS are you on?

[rodrigopedra]

Anyway, with this PR's code, the deferred closure gets executed after the calling artisan command ends, as I thought it was the expected behavior.

I couldn't find in the docs (neither in Laravel's nor in Symfony's) a way to pass additional shell instructions like 2>&1, or & to a process

[rodrigopedra]

Linux (openSUSE Leap 15.6)

[rodrigopedra]

Did you check the logs output? On the CLI no errors are shown

[rodrigopedra]

@taylorotwell

laravel-2024-09-16_12.02.49.mp4

Just got back at my office, recorded a screencast showing the behavior

[taylorotwell]

For what it's worth, we already use the 2>&1 syntax in the console scheduler:

[taylorotwell]

Will go ahead and merge this - though it will keep the FPM worker around while the deferred stuff executes which wasn't totally my intention.

[rodrigopedra]

@taylorotwell I saw this on the codebase, but the Console/Scheduling component executes the command using Process::fromShellCommandline() instead of passing the command as an array, and thus, the 2>&1 & doesn't get quoted.

framework/src/Illuminate/Console/Scheduling/Event.php

Lines 290 to 295 in 79fbf4a

	protected function execute($container)
	{
	return Process::fromShellCommandline(
	$this->buildCommand(), base_path(), null, null, null
	)->run();
	}

Maybe we can change the Concurrency\ProcessDriver to use the same approach. If you want to give it a try, I can send a new PR.

I guess it is better to truly send the deferred commands to the background, so it doesn't keep an FPM worker busy.