Skip to content

[Bug] process-queue-sequential: end a run that finds the command already running without exit() - #722

Open
cancan101 wants to merge 1 commit into
pimcore:2026.3from
cancan101:fix/sequential-queue-already-running-no-exit
Open

cancan101 wants to merge 1 commit into
pimcore:2026.3from
cancan101:fix/sequential-queue-already-running-no-exit

Conversation

@cancan101

Copy link
Copy Markdown
Contributor

Changes in this pull request

Fixes pimcore/platform-version#714

When another process holds the command's lock, SequentialProcessQueueCommand::execute() prints The command is already running. and calls exit(1). exit() inside Command::run() ends the process before the Application dispatches console.terminate. As a result, listeners never see the run end: a cron monitor (Sentry Crons) that closes its check-in on terminate reports every overlapping cron run as a timeout during a long import. The command also can't be run in-process.

It now prints the same message and returns Command::SUCCESS. The run had nothing to do because another process is already working through the queue, and Symfony's LockableTrait example handles the same case the same way. Nothing else changes; the lock was never acquired on this path, so there is nothing to release.

datahub:data-importer:process-queue-parallel reaches the same exit(1) through Pimcore\Console\Traits\Parallelization, which is fixed in pimcore/pimcore#19622 (pimcore/platform-version#713). No change is needed in this repository for that one.

How to reproduce

  1. Start datahub:data-importer:process-queue-sequential while another run holds its lock (or hold the lock yourself: Pimcore::getContainer()->get(LockFactory::class)->createLock('datahub:data-importer:process-queue-sequential', 60)->acquire(false)).
  2. Run the command with a console.terminate listener, or in-process with setAutoExit(false).

Before: the process exits with status 1 inside execute(), and the listener never runs. After: ERROR: The command is already running., exit code 0, and the listener runs.

Additional info

Checked on our install (data-importer 2.4.5 with this change applied as a patch; the code is identical to 2026.3). The command runs through a real Application while a FlockStore lock with its name is held. Before the change the process exits with status 1 and console.terminate is never dispatched. After it, execute() returns 0, console.terminate is dispatched with exit code 0, and the queue is not touched. I haven't added a Codeception test: the suite needs the full Pimcore stack, and the path is a single return.

🤖 Generated with Claude Code

…ady running without exit()

SequentialProcessQueueCommand::execute() called exit(1) when another process
held the command's lock. exit() inside Command::run() ends the PHP process
before the Application dispatches console.terminate, so listeners never see
the run end - e.g. a cron monitor's closing check-in is never sent and every
overlapping cron run is reported as timed out - and the command cannot be
run in-process.

It now returns Command::SUCCESS after printing the same message: the run has
nothing to do because another one is processing the queue, as in the example
of Symfony's LockableTrait. process-queue-parallel has the same exit(1)
through Pimcore's Parallelization trait, fixed in pimcore/pimcore.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 28, 2026 04:39
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are recorded for the focused lock-contention change intended to preserve Symfony’s command lifecycle.

No open issues 🎉

What changed in this PR

The PR addresses overlapping sequential queue runs exiting PHP before Symfony can dispatch console.terminate. It fixes the command-owned early-exit path rather than working around it in monitoring code (src/Command/SequentialProcessQueueCommand.php:60–65).

Changes:

  • Replaces exit(1) with return self::SUCCESS when the lock is unavailable.
  • Preserves the existing message and adds a comment explaining why normal command completion matters.

The observable compatibility change is intentional: lock contention now returns status 0 instead of terminating with status 1; the method signature is unchanged. The fix is local to the sequential command; parallel-command handling is outside this diff.

Runtime event dispatch was not independently verified. No automated regression test or documentation/changelog file is included; the PR description reports manual verification and explains the behavior change.

File Description
src/​Command/​SequentialProcessQueueCommand.php Returns success on lock contention so Symfony can complete the command lifecycle.

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants