Without any testing instructions this is unlikely to get tested very quickly

I have added a test instruction in first comment.

Thank you

Feel free to test.

I have tested this item ✅ successfully on c41eda0

I have tested this item ✅ successfully on b85ddca

Thanks for tests.

First test was done before 'resolve merge conflict' but 'conflicted' merged PR 13372 only change " to '.
Can we add RTC as we have two success tests.

@csthomas We should have another test, please, since we had new commits after the test of @franz-wohlkoenig.
If he can test it again, it would be great

will do

I have tested this item ✅ successfully on b85ddca

thanks

I have tested this item ✅ successfully on b85ddca

RTC

Changing r+b to c+b appears to be responsible for #15544

I'm checking it

Thanks :)

I see a few solution.
The reason is that the cache create a lock before call view method.
If view method generete 404 then process call error page and die.
Cache method does not get any response about failed view.
Cache file is empty and stay to the next requests.

1. First solution is dirty, because because use registered shutdown function to delete such cache file.

diff --git a/libraries/joomla/cache/storage/file.php b/libraries/joomla/cache/storage/file.php
index f2dba86e5f..c4d01e4ed7 100644
--- a/libraries/joomla/cache/storage/file.php
+++ b/libraries/joomla/cache/storage/file.php
@@ -361,6 +361,19 @@ class JCacheStorageFile extends JCacheStorage
                {
                        // Remember resource, flock release lock if you unset/close resource
                        $this->_locked_files[$path] = $_fileopen;
+
+                       $pointer =& $this->_locked_files[$path];
+
+                       // Delete file on unexpected die.
+                       $deleteMe = function () use ($pointer, $path)
+                       {
+                               if ($pointer)
+                               {
+                                       unlink($path);
+                               }
+                       };
+
+                       register_shutdown_function($deleteMe);
                }

2. Second solution is simple, does not delete such file but only check file size:

diff --git a/libraries/joomla/cache/storage/file.php b/libraries/joomla/cache/storage/file.php
index f2dba86e5f..ac0f54ccad 100644
--- a/libraries/joomla/cache/storage/file.php
+++ b/libraries/joomla/cache/storage/file.php
@@ -413,7 +413,7 @@ class JCacheStorageFile extends JCacheStorage
                {
                        $time = @filemtime($path);
 
-                       if (($time + $this->_lifetime) < $this->_now || empty($time))
+                       if (($time + $this->_lifetime) < $this->_now || empty($time) || filesize($path) === 0)
                        {
                                @unlink($path);

I will create a PR for above option 2.

Other solutions may require to catch each error as exception in the cache method and then delete the cache file or try to cache error response.

Hi @csthomas I have experienced another issue due to 'r+b' -> 'c+b' replacement.
Before this change, for a module included in an article with {loadmodule}, it was possible to call JCache::setCaching(false) to disable caching of the container article.
Now an empty file remains in the directory instead, which prevents subsequent view of the article until the cache expires.

Therefore I ask myself if 'c+b' is really necessary? What's the advantage of creating an empty file when you still don't need it?
Otherwise I'd suggest to restore the previous mode 'r+b'.

About creating an empty file.

• The empty file is required to lock this file by flock(), to prevent other php processes to create the same cache file in the same time.

About your situation I understand that:

1. The cache is enabled.
2. There is a request for article view.
3. The cache library tries to find a cache file but there is not file.
4. The cache library creates an empty file and locks it.
5. The cache library calls $view->display()
6. The module loaded by {loadmodule} disables the cache by JCache::setCaching(false)
7. The cache library gets the data but does not store it.
8. The empty cache file left.
9. At the next request the empty cache file exists and there is the problem.

Take a look at #15592 it should resolve your problem.

@csthomas your description is so accurate that for a second, I was afraid that you control my PC with a malware. ?

I understand the need for file locking, but this way we have changed so deeply the behaviour of the file cache, that the problem reported up to now are just the first in a long line that I'm sure will be reported.

I think that the current implementation has a major architectural problem that was already there before your PR, and that can be resumed in "do not lock for longer than is strictly necessary".
Since we currently:

1. lock the file 2 hours and a halt before we are ready to write data into it, and
2. give controls to third party components and modules in the meantime,

then it is reasonable to expect that something will go wrong.
Your attempt to fix it using the shutdown function is clever, but as you said, it's dirty.

To me it was badly designed before your PR, but the 'r+b' mode did prevent early lock.
It's one of those cases where two errors compensates each other.
Having changed the mode to 'c+b' has highlighted the problem.

I just wanted to know if you are agree with that, and I'll start a full refactoring.