Skip to content

Commit

Permalink
Merge pull request #16435 from craftcms/bugfix/asset-indexing-session…
Browse files Browse the repository at this point in the history
…-infinite-loop

prevent infinitely calling `processIndexSession` when `assetindexdata` is empty
  • Loading branch information
brandonkelly authored Jan 15, 2025
2 parents aa43317 + 729a0b0 commit 3c999a1
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG-WIP.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
- Added `craft\helpers\Image::EXIF_IFD0_ROTATE_180_MIRRORED`.
- Added `craft\helpers\Image::EXIF_IFD0_ROTATE_270_MIRRORED`.
- Added `craft\helpers\Image::EXIF_IFD0_ROTATE_90_MIRRORED`.
- Added `craft\models\AssetIndexingSession::$forceStop`. ([#16435](https://github.com/craftcms/cms/pull/16435))
- `GuzzleHttp\Client` is now instantiated via `Craft::createObject()`. ([#16366](https://github.com/craftcms/cms/pull/16366))
- `craft\helpers\DateTimeHelper::humanDuration()` now has a `$language` argument. ([#16332](https://github.com/craftcms/cms/pull/16332))

Expand All @@ -31,4 +32,5 @@
- Fixed a bug where `craft\config\GeneralConfig::safeMode()` set Safe Mode to `false` by default.
- Fixed a bug where Craft wasn’t auto-rotating or flipping images uploaded with a mirrored EXIF orientation.
- Fixed a bug where elements weren’t getting returned in a consistent order when `orderBy` was set to `RAND(X)` across varying `limit` param values. ([#16432](https://github.com/craftcms/cms/issues/16432))
- Fixed a bug where asset indexing could get stuck in an infinite loop if the index data was deleted. ([#16435](https://github.com/craftcms/cms/pull/16435))
- Updated Twig to 3.15. ([#16207](https://github.com/craftcms/cms/discussions/16207))
8 changes: 8 additions & 0 deletions src/controllers/AssetIndexesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ public function actionProcessIndexingSession(): Response
if (!$indexingSession->actionRequired) {
$indexingSession = $assetIndexer->processIndexSession($indexingSession);

if ($indexingSession->forceStop) {
$assetIndexer->stopIndexingSession($indexingSession);
return $this->asFailure(data: [
'stop' => $sessionId,
'message' => Craft::t('app', 'There was a problem indexing assets.'),
]);
}

// If action is now required, we just processed the last entry
// To save a round-trip, just pull the session review data
if ($indexingSession->actionRequired) {
Expand Down
6 changes: 6 additions & 0 deletions src/models/AssetIndexingSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,10 @@ class AssetIndexingSession extends Model
* @since 4.4.0
*/
public bool $processIfRootEmpty = false;

/**
* @var bool Whether we should stop processing the session because there was a problem.
* @since 4.14.0
*/
public bool $forceStop = false;
}
20 changes: 20 additions & 0 deletions src/services/AssetIndexer.php
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,26 @@ public function processIndexSession(AssetIndexingSession $indexingSession): Asse

// The most likely scenario is that the last entry is being worked on.
if (!$indexEntry && !$indexingSession->processIfRootEmpty) {
// if indexEntry is null here, we should also check if there's anything in the assetindexdata table at all
// (if not, it could have been deleted when clearing caches)
// if that table is empty, we'll get into an infinite loop, calling processIndexSession with the same data all the time
// (and it'll be very hard to discard the session via ui)
if ($indexingSession->processedEntries < $indexingSession->totalEntries) {
$count = (new Query())
->from([Table::ASSETINDEXDATA])
->where([
'sessionId' => $indexingSession->id,
'completed' => false,
'inProgress' => false,
])
->count();

if ((int)$count === 0) {
Craft::warning('The assetindexdata table is empty; Can’t proceed with indexing.');
$indexingSession->forceStop = true;
}
}

$mutex->release($lockName);
return $indexingSession;
}
Expand Down
1 change: 1 addition & 0 deletions src/translations/en/app.php
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,7 @@
'There was a problem activating the user.' => 'There was a problem activating the user.',
'There was a problem deactivating the user.' => 'There was a problem deactivating the user.',
'There was a problem impersonating this user.' => 'There was a problem impersonating this user.',
'There was a problem indexing assets.' => 'There was a problem indexing assets.',
'There was a problem saving your message.' => 'There was a problem saving your message.',
'There was a problem sending the password reset email.' => 'There was a problem sending the password reset email.',
'There was a problem with uploading the file.' => 'There was a problem with uploading the file.',
Expand Down

0 comments on commit 3c999a1

Please sign in to comment.