? ? Pending

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
26 Apr 2017

Pull Request for Issue #15544

Summary of Changes

Remove empty locked file at shutdown script if cache callback function terminate process on 404.

This is my second PR (previous #15558)

Reason

To save a cache file joomla requires lock method.
To create a lock for cache joomla has to create a file in the cache folder.
If a view method raise an error then cache process is hung up and the file can not be deleted.
The file is left empty.
Next request could find such file and display it as empty article.

Testing Instructions

Take a look at the issue.

Expected result

Error 404

Actual result

Blank component.

Documentation Changes Required

No

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
4.00

avatar csthomas csthomas - open - 26 Apr 2017
avatar csthomas csthomas - change - 26 Apr 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 26 Apr 2017
Category Libraries
avatar csthomas csthomas - change - 26 Apr 2017
Labels Added: ?
avatar csthomas csthomas - change - 26 Apr 2017
The description was changed
avatar csthomas csthomas - edited - 26 Apr 2017
avatar flyingwombats
flyingwombats - comment - 27 Apr 2017

Tested this patch under the conditions laid out in the original issue and I can confirm it now brings back a proper 404 when before it was not.

avatar csthomas
csthomas - comment - 27 Apr 2017

@flyingwombats Thanks.
Can you go to https://issues.joomla.org/tracker/joomla-cms/15592 and click on button "Test this" and mark test as success.

avatar flyingwombats
flyingwombats - comment - 27 Apr 2017

I have tested this item successfully on fb600c2

404 returns correctly


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15592.

avatar flyingwombats flyingwombats - test_item - 27 Apr 2017 - Tested successfully
avatar OctavianC
OctavianC - comment - 28 Apr 2017

I have tested this item successfully on fb600c2

Tested and solves the issue, I can confirm it fixes #15647 as well.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15592.

avatar OctavianC OctavianC - test_item - 28 Apr 2017 - Tested successfully
avatar franz-wohlkoenig franz-wohlkoenig - change - 28 Apr 2017
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 28 Apr 2017

RTC after two successful tests.

avatar wilsonge wilsonge - change - 28 Apr 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-04-28 20:05:15
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 28 Apr 2017
avatar wilsonge wilsonge - merge - 28 Apr 2017
avatar madesign
madesign - comment - 30 Apr 2017

Sorry, i do not understand quit well what i have to do to fix the issue.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 30 Apr 2017

@madesign please read Answer

avatar tampe125
tampe125 - comment - 4 May 2017

Why are we creating files using the c flag? Isn't the r good enough?
For your information, the function register_shutdown_function is not 100% reliable

avatar csthomas
csthomas - comment - 4 May 2017

Why are we creating files using the c flag? Isn't the r good enough?

r does not create a file. No file - no lock.
I have to create a lockfile before I will create a cache.

For your information, the function register_shutdown_function is not 100% reliable

Function register_shutdown_function should be called in almost 100% cases, except fatal errors.
I know it looks weird but callback function can terminate process by exit(0).

May be the best way would be to not block other processes to create a cache data but only block on save the data to cache file.

Old version does not create a lock for not existed cache files.
There was an error because for example: 10 processes can create the same cache file in the same time and then they override the file a few times.

avatar waveywhite
waveywhite - comment - 5 May 2017

I've just tried this fix on PHP 5.5.9 and it is now not saving any page cache files. I was suffering from the "zero length cache file" problem for pages which the page cache plugin ignored. This fix is now removing all cache files.

We're using a custom page cache plugin based on the Joomla one. It was working fine on 3.6.x. I'll also check that there aren't any changes to the 3.7 page cache plugin we should be considering.

avatar csthomas
csthomas - comment - 5 May 2017
avatar csthomas
csthomas - comment - 5 May 2017

I think about redesign it once again and remove cache locking from outside get and store methods.

avatar csthomas
csthomas - comment - 5 May 2017

Can you put fflush($_fileopen); after $result = @fwrite($_fileopen, $data, $length); line 216 in file libraries/joomla/cache/storage/file.php and check if this resolve your problem?

avatar waveywhite
waveywhite - comment - 5 May 2017

OK. Do you mean in the patched file.php or the Joomla 3.7.0 version?

avatar csthomas
csthomas - comment - 5 May 2017

In the patched file

avatar csthomas
csthomas - comment - 5 May 2017

You can also add a few lines below fclose($_fileopen) another function clearstatcache();

avatar waveywhite
waveywhite - comment - 5 May 2017

I've got the following:

                if ($_fileopen)
                {
                        $length = strlen($data);
                        $result = @fwrite($_fileopen, $data, $length);
                        fflush($_fileopen);

                        if ($close)
                        {
                                @fclose($_fileopen);
                        }
                        clearstatcache();

                        return $result === $length;
                }

But it's not helping. There are still no files ending up in cache
On a different server (same site, same OS - Ubuntu 14.04) it's working OK. Could we have a race condition?

avatar csthomas
csthomas - comment - 5 May 2017

Example is OK. Then I have no more ideas now.
One php process should create some cache files. Do you have correct permission and free space on disk?

avatar waveywhite
waveywhite - comment - 5 May 2017

OK, it's working now. It looks like it was trouble at my end which occurred at the exact same time I applied this patch.

It seems that the cache/page folder was recreated in some way, probably via an rsync action, while my terminal's CWD was in that folder. So, when I ls -l I see no cache files. However, several hours later I change directory and voilà, all the files are in the 'new' cache/page folder and it was working all along.

Sorry to waste your time and thank you for your efforts to sort this out.

Add a Comment

Login with GitHub to post a comment