? ? Failure

User tests: Successful: Unsuccessful:

avatar demis-palma
demis-palma
7 May 2017

Definitive solution for “zero length cache file”

that has caused so much troubles in such a short time (and so many problems would have caused in future) #10767 #15544 #15558 #15592

The current implementation of file storage makes use of the shutdown function, but it doesn't work in case of software crashes, hardware failure, power blackout, and so on.

Summary of Changes

  1. Don't create empty cache files at all
  2. Limited the lock for no longer than is strictly necessary to the write operation
  3. Delegated file locking to PHP
  4. Addressed and fixed a couple of other latent cache bugs
  5. Significant code simplification and major performance improvements.
  6. Given resistance and stability against the unlikely event of corrupted cache files (not only zero length)

Here we have all the possible concurrent circumstances, and their consequences before and after this PR.

1. Write followed by a concurrent Write

? Before this PR It depends on how long it takes the first write process. If the first write ends before the second’s time-out, then the concurrent write will be enqueued, otherwise it will be discarded. A big mess.
✔️ After this PR The concurrent write will be enqueued to the first write in any case.

2. Write followed by a concurrent Read

Before this PR Unserialize error. This also causes a PHP warning, which could possibly disclosure path information.
This happens because the caller expects a valid serialized data or null. It does not expect invalid data.
Unlikely to actually happen? For the “Infinite monkey theorem” it will almost surely happen and actually happened in past: https://forum.joomla.org/viewtopic.php?t=851183
✔️ After this PR No error happens. The cache ensures to always return valid data or null, as the caller expect. This is done by introducing an integrity self check to the cache buffer.

3. Read followed by a concurrent Read

✔️ Before this PR No problem
✔️ After this PR No problem

4. Read followed by a concurrent Write

Before this PR It depends on the file size. The most of the times due to the hard drive buffer, the entire file remains available in the drive cache for the reading process even if it has been already overwritten on the disk.
Otherwise, if the file is bigger than the hard drive cache, the cache buffer gone corrupted and it causes unserialize error to the next process that will read the file (see above).
✔️ After this PR The same as before the PR, with the difference that a corrupted file is detected and invalidated by the integrity self check, and it raises no errors. The normal program flux will continue, ignoring the cache for the current http request.

5. Write followed by a concurrent Deletion

Quite scary, but no particular problem. It is behaves as case 1. Write followed by a concurrent Write

6. Read followed by a concurrent Deletion

Quite scary, but no particular problem. It is behaves as case 4. Read followed by a concurrent Write

7-8. Deletion followed by a concurrent Read | Write

Deletion is atomic and does not suffer concurrency from Read or Write operations, so the cases Deletion followed by a concurrent Read | Write don’t exist.

Performance improvements

  1. Do not delete anymore expired files before recreate them. Just overwrite them.
  2. Do not use anymore the str_replace() function searching for the php die protection heading along the entire cache buffer. Used an offset to shift the file pointer + sequential reading instead.

Other changes

Mimic a successful lock and unlock operations to keep compatibility with the previous interfaces, but this storage engine does not need lock / unlock methods

Testing Instructions

I have stressed my system with 100 concurrent requests a a time, and I got no errors.
At the moment, I've tested on Linux only. Windows and OSX should not reserve surprises, but It's better to do a test drive.

avatar demis-palma demis-palma - open - 7 May 2017
avatar demis-palma demis-palma - change - 7 May 2017
Status New Pending
avatar demis-palma demis-palma - edited - 7 May 2017
avatar joomla-cms-bot joomla-cms-bot - change - 7 May 2017
Category Libraries
avatar demis-palma demis-palma - change - 7 May 2017
The description was changed
avatar demis-palma demis-palma - edited - 7 May 2017
avatar demis-palma demis-palma - change - 7 May 2017
The description was changed
avatar demis-palma demis-palma - edited - 7 May 2017
avatar demis-palma demis-palma - change - 7 May 2017
The description was changed
avatar demis-palma demis-palma - edited - 7 May 2017
avatar brianteeman
brianteeman - comment - 7 May 2017

@demis-palma please look at the travis errors

avatar demis-palma demis-palma - change - 7 May 2017
The description was changed
Labels Added: ?
avatar demis-palma
demis-palma - comment - 7 May 2017

@brianteeman sure.

avatar demis-palma demis-palma - edited - 7 May 2017
avatar demis-palma demis-palma - change - 7 May 2017
The description was changed
avatar demis-palma demis-palma - edited - 7 May 2017
avatar demis-palma demis-palma - change - 7 May 2017
The description was changed
avatar demis-palma demis-palma - edited - 7 May 2017
avatar demis-palma demis-palma - change - 7 May 2017
The description was changed
avatar demis-palma demis-palma - edited - 7 May 2017
avatar joomla-cms-bot joomla-cms-bot - change - 8 May 2017
Category Libraries Libraries Unit Tests
avatar csthomas
csthomas - comment - 10 May 2017

In general it is OK.

Changes in lock/unlock methods may have some B/C issue but I do not protest.
Option from cache locking is useless now but is should not be any problem. It should be deprecated.

If this changes can go then other storages can be done in the same way and controllers/adapters can forget about lock/unlock cache storages/handlers in get/store methods.

avatar tampe125
tampe125 - comment - 17 May 2017

I think this is the correct approach to the cache issue. We lock before writing and there is an extensive use of defensive coding.
?

avatar rdeutz rdeutz - change - 27 May 2017
Labels Added: ?
avatar HLeithner
HLeithner - comment - 15 Mar 2022

Similar to #15707

This PR is outdated and have to be updated to fit in to j3. J4 uses a legacy caching system which should be replaced with a 3rd party library compatible PSR-6 / PSR-16 package see https://github.com/joomla-framework/cache

If you are still interested to bring this to joomla we would be happy if you help us working on the new cache layer or if you like you can bring this PR in Sync for 3.10 but since the focus is J4 it would be better to help use with the caching layer.

thanks I'm closing this for now, feel free to reopen.

avatar HLeithner HLeithner - close - 15 Mar 2022
avatar HLeithner HLeithner - change - 15 Mar 2022
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2022-03-15 13:48:53
Closed_By HLeithner
Labels Added: ? ?
Removed: ? ?

Add a Comment

Login with GitHub to post a comment