[4] Ensure opcache file invalidate on file copy & delete

[PhilETaylor]

This is part 2 of #32592 and directed at Joomla 4

Summary of Changes

When PHP is configured to use modern Opcache, PHP will attempt to hold the compiled version cached in memory.

There are many ways to configure opcache invalidations, including never revisiting the source PHP to compile it again ever (if opcache.validate_timestamps is disabled for example)

To fully ensure that files that Joomla writes to the hard disk are taken into account by a server with a correct configured (or badly configured) opcache, we need to invalidate files from the opcache after writing a new version to the disk.

Joomla has previously used opcache_reset sporadically to do this, but this is VERY BAD as causes a WHOLE SERVER cache clear - including other sites, and other sites you might not own! It can also cause spikes in CPU because all those sites need to recompile their opcodes!

Testing Instructions

Hard to test unless you really know what you are doing and can reconfigure all your PHP stack to include opcaching. Also some of the edge cases this fixes will only become clear if you are an extension developer mass distributing extensions.

Install some extensions - nothing should break.

EDIT: Some more details on the testing can be found in comment #32915 (comment)

Actual result BEFORE applying this Pull Request

You can install extensions

Expected result AFTER applying this Pull Request

You can install extensions and opcache for each file is invalidated

Documentation Changes Required

none

Others

WordPress started this conversation 5 years ago and only last year got their changes into core
https://core.trac.wordpress.org/changeset/48160

// cc @nikosdion

[wilsonge]

Sorry to be a pain but can you rename invalidateOpcache to invalidateFileCache or something. Just so we have options in the future if we want to call other cache invalidates

[wilsonge]

Thankyou!

[SharkyKZ]

Not sure why this hasn't been closed yet. As maintainers will explain to you, this PR is wrong.

[joomla-cms-bot]

Set to "closed" on behalf of @SharkyKZ by The JTracker Application at issues.joomla.org/joomla-cms/32915

[joomdonation]

If @SharkyKZ doesn't want to explain what's wrong, at least one of our maintainers should explain what's wrong in this PR. Closing a PR without explaining will hurt our contributors :(

[joomdonation]

@PhilETaylor Maybe @SharkyKZ has explained what's wrong with our maintainers. So please wait for an explanation.

[SharkyKZ]

Its time his rights were removed.

I agree. In fact, all of my rights should have been revoked in November 2020 when I left JBS. But, clearly, someone royally screwed up with my offboarding. Or perhaps didn't even bother at all. I bet if I hadn't manually removed myself from the organization on Github, I'd still have permissions here too. Unauthorized users managing the issue tracker? Sure, no problem! 👍 And this alone perfectly sums the state of the entire project. Too much ignorance on the part of people who run things here. Frankly, Phil, based on your issue history you should understand this better than anyone else.

[bembelimen]

Sorry, that this happened.

[richard67]

Are you sure these issues are related to this PR, and not just problems with the upgrade process? ;-)

@PhilETaylor If I was sure about anything yet, I would not investigate still ;-)

[richard67]

as everything in this PR is pretty much reliant on opcache being enabled, and if not no harm should be caused :)

@PhilETaylor Except of the changes where calls to native PHP functions have been replaced by calls to methods of the File and Folder classes of the Filesystem library and the latter include calls to Path::clean. Such changes may have an effect regardless of opcache.

[richard67]

@PhilETaylor Other question: The proxy function I have added with my PR https://github.com/PhilETaylor/joomla-cms/pull/10 here has only 2 parameters: https://github.com/joomla/joomla-cms/pull/32915/files#diff-d0a9b340bfb7e20bbd4f1c8603bc3efc4eeca71c8cd65b078a6ae4c166f3ad01R131

That's ok as long as in script.php it's used with only 2 parameters.

But the function which is proxied has 2 additional parameters: https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Filesystem/File.php#L264

Shall we add them to the proxy? If so, what to do with the $useStreams? Shall we handle that somehow or just silently ignore it?

[richard67]

@PhilETaylor Regarding my issue: I think you are right and it's not related to your PR. I just want to solve it and finally check if everything works before I give your PR a good test.

[richard67]

Shall we add them to the proxy? If so, what to do with the $useStreams? Shall we handle that somehow or just silently ignore it?

Nics code doesnt use streams so its ok to ignore that. The restoration code, being standalone from Joomla on purpose, should be as compact, and featureless as possible. At the end of the day all it needs to be able to do is extract a zip file without erroring, and then return control to Joomla to cleanup.

@PhilETaylor So what do you finally suggest? Is https://github.com/joomla/joomla-cms/pull/32915/files#diff-d0a9b340bfb7e20bbd4f1c8603bc3efc4eeca71c8cd65b078a6ae4c166f3ad01R131 ok as it is now, or shall I add the other 2 parameters, too, handle the $path parameter right and ignore the $useStreams, in order to have the same signature and in case if someone in future wants call File::move in script.php with such parameters?

[richard67]

@PhilETaylor My last issue was definitely caused by my testing environment. It could have been some sticky bit having been set by mistake on some folders, together with wrong group maybe. No idea how that could happen. Am just finishing a few tests for the installer. Looks good so far with opcache on and also with off.

[richard67]

Then let them add them then at that point. File::move in the update process is not meant to be a fully featured API like the one in the Filesystem namespace. Its mean to be a minimised version that can run standalone to Joomla, for the purposes of extracting the zip file only.

Agree.

[richard67]

I have tested this item ✅ successfully on 15ae8a2

Have tested on diverse environments:

• IONOS shared hosting on Linux with opcache enabled (file cache only)
• Linux without opcache enabled
• Windows without opcache enabled

The renaming of files with wrong case in script.php still works on Linux and Windows.

Installing, updating and uninstalling extensions works.

I'm still worried about certain thumbs down, maybe I've missed something. But as the author of the thumbs down will not explain the reason there is nothing I can do about that.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32915.}

[richard67]

@wilsonge I've tested the rename stuff on Linux on Windows. Would be cool if you could check on IOS because that was the most weird one.

[richard67]

I'm still worried about certain thumbs down, maybe I've missed something. But as the author of the thumbs down will not explain the reason there is nothing I can do about that.

The thumbs down from my stalker were applied moments after this PR was first opened, it should be noted that this PR has been drastically improved since then though 44 revisions.

@PhilETaylor Yes, that's my hope, that the reasons for it were fixed on the way.

[wilsonge]

@PhilETaylor can we just now change the installation to use the Filesystem method in the PR please https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/src/Model/CleanupModel.php#L33-L46

[wilsonge]

Thanks!