Skip to content

Fix crash on PHP 8.3 when a file is missing #20358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix crash on PHP 8.3 when a file is missing #20358

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
690f9b0
Fix crash on PHP 8.3 when a file is missing
Apr 21, 2025
0c2d6b3
Fix Warning when file is missing
Jun 18, 2025
dd9bc77
Fix code style - add space
Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion framework/caching/FileCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ protected function getValue($key)
{
$cacheFile = $this->getCacheFile($key);

if (@filemtime($cacheFile) > time()) {
if (file_exists($cacheFile) && @filemtime($cacheFile) > time()) {
Copy link
Member

Choose a reason for hiding this comment

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

It is a bit weird, cause @ should've suppressed the error. Any idea why it doesn't work?

Introducing file_exists make the operation non-atomic which isn't great if we're talking about cache.

A more universal way would've been something like how it is done in Yii3...

Copy link
Author

Choose a reason for hiding this comment

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

I looked into the issue a bit deeper. In my Craft CMS setup, I had DEV_MODE=true enabled. In this mode, a PHP Warning interrupts program execution just like a PHP Error. So, the problem with the missing cache file must be happening at a different level.

Could you clarify what exactly makes the cache file operations non-atomic in this case? Yes, there are now two file operations: checking if the file exists and checking its modification time. But isn’t that the whole point of this method? First, verify the file exists, and only if it does, check its modification time. With the double check, we’d simply exit at the first step if the file isn’t there.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you first check if file exist, and then read mtime from it, this means that file could be deleted by other process between these two operations. So file_exists doesn't completely protect you from such errors, it only makes them appear much less frequently (at the cost of additional call, which is not necessary in 99.99% setups).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @XOlegator

I had DEV_MODE=true enabled. In this mode, a PHP Warning interrupts program execution just like a PHP Error

If this is the case, isn't Yii behaving normal here? The method filemtime generates a warning when file does not exist which results in a crash in dev mode. Can you explain where Yii goes wrong?

Copy link
Author

Choose a reason for hiding this comment

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

You're absolutely right that filemtime() itself behaves as expected by emitting a warning when a file doesn't exist. The core issue isn’t with Yii’s behavior, but rather with error handling in contexts like Craft CMS’s DEV mode, where warnings are treated as fatal exceptions.
Is Yii "Wrong"? Not at all! This is more about ecosystem compatibility. Many modern PHP frameworks/libraries treat warnings as strict-mode errors. The change simply makes Yii more resilient in such environments without sacrificing performance.

$fp = @fopen($cacheFile, 'r');
if ($fp !== false) {
@flock($fp, LOCK_SH);
Expand Down