Skip to content

Catch IOException in CacheCleaner to prevent upgrade abort on locked cache files - #1820

Open
rboadalabelgrup wants to merge 1 commit into
PrestaShop:7.6.xfrom
rboadalabelgrup:fix/cache-cleaner-catch-ioexception-locked-files
Open

Catch IOException in CacheCleaner to prevent upgrade abort on locked cache files#1820
rboadalabelgrup wants to merge 1 commit into
PrestaShop:7.6.xfrom
rboadalabelgrup:fix/cache-cleaner-catch-ioexception-locked-files

Conversation

@rboadalabelgrup

Copy link
Copy Markdown

During an upgrade, if PHP-FPM processes have cache files open, clearDirectory() throws an IOException and the upgrade process aborts completely.

This change wraps the clearDirectory() call in a try/catch so that locked files produce a warning in the log instead of an unhandled exception, allowing the upgrade to continue.

Questions Answers
Description? During an upgrade, if PHP-FPM processes have cache files open, clearDirectory() throws an IOException and the upgrade process aborts completely. This change wraps the clearDirectory() call in a try/catch so that locked files produce a warning in the log instead of an unhandled exception, allowing the upgrade to continue.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? N/A
Sponsor company N/A
How to test? 1. Run a PrestaShop instance with PHP-FPM
2. Start an upgrade while PHP-FPM processes have cache files open
3. Before: the upgrade aborts with an unhandled IOException
4. After: the upgrade continues and a warning appears in the log instead

Possible impacts

  • Cache clearing behavior during upgrades
  • Upgrade log output (a new warning message may appear when cache files are locked)

@ps-jarvis

Copy link
Copy Markdown
Collaborator

Hello @rboadalabelgrup!

This is your first pull request on autoupgrade repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

@github-project-automation github-project-automation Bot moved this to Ready for review in PR Dashboard May 27, 2026
@Quetzacoalt91

Copy link
Copy Markdown
Member

Can you please target the branch 7.6.x and rebase from there?

About the changes, the changes look good to me but as this is not an actual fix and the cache partially remains, this will require some tests before being approved by the QA.

@rboadalabelgrup
rboadalabelgrup force-pushed the fix/cache-cleaner-catch-ioexception-locked-files branch from 33c73e6 to b34df61 Compare May 28, 2026 13:47
@rboadalabelgrup
rboadalabelgrup changed the base branch from dev to 7.6.x May 28, 2026 13:48
@rboadalabelgrup

Copy link
Copy Markdown
Author

Can you please target the branch 7.6.x and rebase from there?

About the changes, the changes look good to me but as this is not an actual fix and the cache partially remains, this will require some tests before being approved by the QA.

Agreed, this is a best-effort approach. The upgrade continues and the operator is warned via the log. If a stricter behavior is preferred (e.g. retrying or listing the locked files), I'm open to suggestions.

@Quetzacoalt91

Copy link
Copy Markdown
Member

We also lack actual solution on that. Our investigations showed that any parallel request to the store while the update is running will generate the cache. If the deletion of the cache happens at the same time, we will encounter the collision.
We have limited the risk with #1523, but the issue didn't disappear.

@rboadalabelgrup

Copy link
Copy Markdown
Author

@Quetzacoalt91 Thanks for the context! I had a look at #1523 — makes sense as a first line of defense for back office AJAX requests. My change would cover the cases that guard can't block, like requests coming from the front end or external APIs. The upgrade wouldn't abort in those situations, the locked files would just remain and the operator gets a warning in the log. Not a perfect solution, but it could complement what's already there. Happy to adjust if you have something else in mind.

@Quetzacoalt91

Copy link
Copy Markdown
Member

I've the updated the base branch so the CI does not fail for a missing library canvas. Can you please rebase?

@rboadalabelgrup
rboadalabelgrup force-pushed the fix/cache-cleaner-catch-ioexception-locked-files branch from b34df61 to 45d6906 Compare June 1, 2026 14:12
@rboadalabelgrup

Copy link
Copy Markdown
Author

Done, rebased on 7.6.x.

@sonarqubecloud

sonarqubecloud Bot commented Jun 1, 2026

Copy link
Copy Markdown

@ps-jarvis ps-jarvis added the Waiting for QA Status: Action required, Waiting for test feedback label Jun 11, 2026
@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard Jun 11, 2026
@Quetzacoalt91 Quetzacoalt91 added this to the 7.6.6 milestone Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Waiting for QA Status: Action required, Waiting for test feedback

Projects

Status: To be tested

Development

Successfully merging this pull request may close these issues.

3 participants