Bug #87312

Scheduler fails at ConsoleCommand at the same time with the same idenitifer

Added by Guido Schmechel over 1 year ago. Updated about 1 month ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
scheduler
Target version:
-
Start date:
2018-12-31
Due date:
% Done:

100%

TYPO3 Version:
9
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

Context:

  • Given is a Symfony console command registered with "test:job"
  • This job has one required argument "message"
  • Activate two tasks in the scheduler, e.g. every minute
  • TYPO3 Cronjob runs every minute, too (CLI)
  • Task 1 registered with argument "hello", Task 2 registered with argument "world"

Problem:

  • TYPO3 processes the jobs sequential
  • First call is "scheduler:run" via CLI
  • Second call is our Task 1 (hello)
  • vendor/symfony/console/Command/Command.php has no "command". Here is the magic to add these "command". So we have now "test:job" as command and "hello" as message. Job runs successfully.
  • Third call (Task 2) has only the argument "message" (world) and TYPO3 runs the job without the "command" argument
  • The Job exit with code 255 and save the trace into the database field

Error message

a:5:{s:4:"code";i:0;s:7:"message";s:42:"Not enough arguments (missing: "command").";s:4:"file";s:66:"project\vendor\symfony\console\Input\Input.php";s:4:"line";i:76;s:11:"traceString";s:2742:"#0 project\vendor\symfony\console\Input\Input.php(42): Symfony\Component\Console\Input\Input->validate()
#1 project\vendor\symfony\console\Input\ArrayInput.php(34): Symfony\Component\Console\Input\Input->__construct(Object(Symfony\Component\Console\Input\InputDefinition))
#2 project\public\typo3\sysext\scheduler\Classes\Task\ExecuteSchedulableCommandTask.php(88): Symfony\Component\Console\Input\ArrayInput->__construct(Array, Object(Symfony\Component\Console\Input\InputDefinition))
#3 project\public\typo3\sysext\scheduler\Classes\Scheduler.php(180): TYPO3\CMS\Scheduler\Task\ExecuteSchedulableCommandTask->execute()
#4 project\public\typo3\sysext\scheduler\Classes\Command\SchedulerCommand.php(145): TYPO3\CMS\Scheduler\Scheduler->executeTask(Object(TYPO3\CMS\Scheduler\Task\ExecuteSchedulableCommandTask))
#5 project\public\typo3\sysext\scheduler\Classes\Command\SchedulerCommand.php(99): TYPO3\CMS\Scheduler\Command\SchedulerCommand->loopTasks()
#6 project\vendor\symfony\console\Command\Command.php(255): TYPO3\CMS\Scheduler\Command\SchedulerCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#7 project\vendor\symfony\console\Application.php(901): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#8 project\vendor\symfony\console\Application.php(262): Symfony\Component\Console\Application->doRunCommand(Object(TYPO3\CMS\Scheduler\Command\SchedulerCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 project\vendor\symfony\console\Application.php(145): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#10 project\public\typo3\sysext\core\Classes\Console\CommandRequestHandler.php(63): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 project\public\typo3\sysext\core\Classes\Console\CommandApplication.php(48): TYPO3\CMS\Core\Console\CommandRequestHandler->handleRequest(Object(Symfony\Component\Console\Input\ArgvInput))
#12 project\public\typo3\sysext\core\bin\typo3(23): TYPO3\CMS\Core\Console\CommandApplication->run()
#13 project\public\typo3\sysext\core\bin\typo3(24): {closure}()
#14 {main}";}

Associated revisions

Revision 77a3e843 (diff)
Added by Helmut Hummel about 2 months ago

[BUGFIX] Skip input validation in scheduler command task

All commands that are added via Configuration/Commands.php
are instantiated and prepared when the "scheduler:run" command is
executed. This preparation includes setting the Application object.

If this Application object is present in an Command object,
calling the run method of the command will cause a merge
of the application's arguments with command arguments.

Since command objects in symfony/console and TYPO3 are
more or less singletons, these merged arguments will
stay merged on a second "instantiation" of the command
through when scheduler executes the second task with the same command.

Therefore we must SKIP the validation of the new Input object
we create and pass to the command's run method, so that this
validation does not fail due to the missing command argument.

It also does not matter that we skip validation, because validation
happens implicitly when calling the run method on the command object.
Only Symfony code is "smart" enough to set the command argument,
when it was not present before.

Resolves: #87312
Releases: master,9.5
Change-Id: I75677493216961fd7559a923e94e7831e07921bf
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/63964
Tested-by: Helmut Hummel <>
Tested-by: TYPO3com <>
Tested-by: Markus Klein <>
Tested-by: Richard Haeser <>
Reviewed-by: Helmut Hummel <>
Reviewed-by: Benni Mack <>
Reviewed-by: Markus Klein <>
Reviewed-by: Manuel Selbach <>
Reviewed-by: Andreas Fernandez <>
Reviewed-by: Richard Haeser <>

Revision a3730417 (diff)
Added by Helmut Hummel about 2 months ago

[BUGFIX] Skip input validation in scheduler command task

All commands that are added via Configuration/Commands.php
are instantiated and prepared when the "scheduler:run" command is
executed. This preparation includes setting the Application object.

If this Application object is present in an Command object,
calling the run method of the command will cause a merge
of the application's arguments with command arguments.

Since command objects in symfony/console and TYPO3 are
more or less singletons, these merged arguments will
stay merged on a second "instantiation" of the command
through when scheduler executes the second task with the same command.

Therefore we must SKIP the validation of the new Input object
we create and pass to the command's run method, so that this
validation does not fail due to the missing command argument.

It also does not matter that we skip validation, because validation
happens implicitly when calling the run method on the command object.
Only Symfony code is "smart" enough to set the command argument,
when it was not present before.

Resolves: #87312
Releases: master,9.5
Change-Id: I75677493216961fd7559a923e94e7831e07921bf
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64143
Tested-by: TYPO3com <>
Tested-by: Richard Haeser <>
Reviewed-by: Richard Haeser <>

History

#1 Updated by Guido Schmechel over 1 year ago

  • Description updated (diff)

#2 Updated by Guido Schmechel over 1 year ago

This isn't a real fix, just a short workaround

typo3/sysext/scheduler/Classes/Task/ExecuteSchedulableCommandTask.php Line 89

        $definition = $schedulableCommand->getDefinition();
        if ($definition->hasArgument('command') && !array_key_exists('command', $this->getArguments())) {
            $this->arguments = array_merge($this->getArguments(), ['command' => $schedulableCommand->getName()]);
        }

Unfortunaly i don't know why the command info is lost in a loop scenario.

#3 Updated by Guido Schmechel over 1 year ago

  • Description updated (diff)

#4 Updated by Riccardo De Contardi over 1 year ago

  • Category set to scheduler

#5 Updated by Susanne Moog 3 months ago

  • Status changed from New to Needs Feedback

I don't quite get what I have to do to reproduce the issue. Can you provide your test command and scheduler configuration here?

#6 Updated by Armin Vieweg 2 months ago

Hi. I also can reproduce this issue.
You just need to create a scheduler task for a Symfony command.
Then create a second task, using the same command (with e.g. different arguments).

When you run both separately, it works.

When you run both together, the first running one works, but the second one get an additional argument:

  ["command"]=>
  object(Symfony\Component\Console\Input\InputArgument)#775 (4) {
    ["name":"Symfony\Component\Console\Input\InputArgument":private]=>
    string(7) "command" 
    ["mode":"Symfony\Component\Console\Input\InputArgument":private]=>
    int(1)
    ["default":"Symfony\Component\Console\Input\InputArgument":private]=>
    NULL
    ["description":"Symfony\Component\Console\Input\InputArgument":private]=>
    string(22) "The command to execute" 
  }

Which is required and causes this issue.

#7 Updated by Susanne Moog 2 months ago

  • Status changed from Needs Feedback to Accepted

#8 Updated by Armin Vieweg 2 months ago

I think the reason is the \Symfony\Component\Console\Command\Command::mergeApplicationDefinition method, which get called in ->run() method.

In CommandRegistry a new instance of the Command is created, but (apparently) not for its definition.

#9 Updated by Gerrit Code Review 2 months ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63964

#10 Updated by Gerrit Code Review 2 months ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63964

#11 Updated by Gerrit Code Review 2 months ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63964

#12 Updated by Jonas Eberle about 2 months ago

Is there an easy way to reproduce with just default commands and the scheduler?

#13 Updated by Helmut Hummel about 2 months ago

Armin Vieweg wrote:

I think the reason is the \Symfony\Component\Console\Command\Command::mergeApplicationDefinition method, which get called in ->run() method.

Sorry, I don't understand this conclusion. The command object is run calling the run method, but there isn't an Application object set in the command, which will cause mergeApplicationDefinition to exit early.

#14 Updated by Helmut Hummel about 2 months ago

  • Status changed from Under Review to Needs Feedback

I tried to reproduce with TYPO3 master and sf/console 4.4 and 5.0 but failed to get the error, because the application object is never added to the command, thus the apllication definition is never merged.
And I have no clue how it can happen the command objects have the application object already set.

#15 Updated by Gerrit Code Review about 2 months ago

  • Status changed from Needs Feedback to Under Review

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63964

#16 Updated by Gerrit Code Review about 2 months ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63964

#17 Updated by Helmut Hummel about 2 months ago

The issue is mitigated on master branch, when all commanda are registered via DI.
The mitigation works, because \TYPO3\CMS\Core\Console\CommandRegistry implements \Symfony\Component\Console\CommandLoader\CommandLoaderInterface in master and
commands that are registered via DI are lazily instantiated. Lazy means, only when the command itself is called, or the list command is called.

Commands registered via Configuration/Commands.php are always instantiated whether they are executed or not. This means, when the scheduler:run command is executed all other commands
are prepared as well and part of this preparation is adding the Application object to each command object.
If the Application object is present though, the first execution of the run method, merges the Application's definition with the one from the command.
This isn't an issue, because the run method itself takes care to set the command argument properly.

However our code that is executed before calling this method, prepares the Input and passes the Command's Definition in this preparation, which in turn
leads to a validation of the Imput. This validation however fails, when the command object already has the command argument merged from the Application.

#18 Updated by Gerrit Code Review about 2 months ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63964

#19 Updated by Gerrit Code Review about 2 months ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/64143

#20 Updated by Helmut Hummel about 2 months ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#21 Updated by Benni Mack about 1 month ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF