Marking this as a release blocker for the next stable as this is required for 3.10 to 4.1 updates. ;)

I'm not sure about the 6 seconds, That sounds very long to me specificly as this will be taken up by any update. Woudnt 2 or 3 seconds be ok too? Given that on some hosts we "just" have the 30 seconds max excecution time reserving 6 seconds sonds very long to me.

"Just" passing to the JS whether we are running opcache or not is also not as easy as it sounds for an non-js guy right?

Hmm, I don't care so much about the 6 seconds. Sometimes on a very fast environment, the update was that fast that I thought something is wrong, so maybe the 6 seconds help people on fast environments to see that something is happening ? .

But opinions may differ.

Given that on some hosts we "just" have the 30 seconds max excecution time reserving 6 seconds sonds very long to me.

this is pausing time in the browser so it should not affect the max executing time of the server. Anyways the number is totally random, could be less could be more

"Just" passing to the JS whether we are running opcache or not is also not as easy as it sounds for an non-js guy right?

I have no clue how to sniff if OPCache is enabled but I guess if there's a fn then in PHP we could:

Factory::getDocument()->addScriptOptions('opcache', json_encode(is_array(opcache_get_status()));

and then in the JS world we could just pick the value:

var opcacheState = Joomla.getOptions('opcache');

if (opcacheState){
  setTimeout(finaliseUpdate, 6000);
} else {
finaliseUpdate();
}

But too much code for no obvious gain. I'll say in this case set always a reasonable timeout, less code, less bugs...

Maybe we could show a nice 1990s style animation during the 6 seconds ?

Maybe we could show a nice 1990s style animation during the 6 seconds

@dgrammatiko A first test on the environment where I could reproduce the issue with default opcache settings was successful. I will test more to make sure it was not lucky circumstance.

this is pausing time in the browser so it should not affect the max executing time of the server. Anyways the number is totally random, could be less could be more

Maybe a dump question because i dont understand JS but what happens on the server at that time? Isnt the request on the server still running but "just" not getting forward in the client? Or is the request stopped on the server and after the 6 seconds a new request started?

I have no clue how to sniff if OPCache is enabled but I guess if there's a fn then in PHP we could:

On a quick look, not tested yet it should work using this https://www.php.net/manual/de/function.ini-get.php asking for "opcache.enable" https://www.php.net/manual/en/opcache.configuration.php

Maybe a dump question because i dont understand JS but what happens on the server at that time? Isnt the request on the server still running but "just" not getting forward in the client? Or is the request stopped on the server and after the 6 seconds a new request started?

The update process is a sequence of 3-4 Ajax requests. The pausing is happening between the last two requests (or so). Each request is bound to server's max execution time only for the response, eg we make the request and the server has up to, let's say, 30 seconds to do what we requested or it will time out. In between those requests the server is idle, not doing any work because there's no request to process. We added some wait time in between those requests, therefore the server should be unaware of that time and also unaffected.

Ok thanks @dgrammatiko sounds good to me.

I have tested this item ✅ successfully on a09a21f

I could reliably reproduce that without this PR and opcache enabled with default settings for PHP 7.4, the issue happens, and with this PR it does not happen.

Important hint for testers: Clear your browser cache after each test so you can be sure that the right version of the js is used.

@zero-24 @bembelimen @wilsonge The change from this PR shall not be merged up into 4.x-dev. It is only needed for 3.10-dev for updates from 3.10 to 4. Later when updating from 4.0.x or 4.1.x versions, it is not needed because on the 4.x versions we have a different opcache handling which doesn't need this - let me call it - workaround.

well... well... well... come back to be bit in the arse...

Abandoning this for Joomla 3.
Joomla announced that Joomla 3.9.24 was the last version of Joomla 3 to be released (despite 3.9.25 and 3.9.26 already here, and a milestone for 3.9.27 already starting to gather issues), there is no point fixing this in Joomla 3 - but lets get it right for Joomla 4.

#32918

try testing that PR and see if that PR fixes the issue.. if its still can be merged ok..

@PhilETaylor Just by reading the lines added to your pr I’ll dare to say it won’t work as the FILE class is mocked (never loaded) in the com_joomlaupdate/restore.php. The restore.php is an application on its own and doesn’t load anything from the libraries folder

ah yeah we only added that in Joomla 4 version - see this change:

https://github.com/joomla/joomla-cms/pull/32915/files#diff-87944bb26710d00cc11ee21e5a9c9242c5fd957acade50f9eed59b84f87b563b

maybe that can be back ported.

@zero-24 actually @PhilETaylor ‘s version is a proper solution (assuming the code is added in the restore.php) thus I suggest to follow that path here. @PhilETaylor could you create a pr with the changes?
If we all agree with Phil’s solution I will close this one

Any changes to the restore.php have to be done carefully as we all know. Hopefully we can also get a review of the changes by Nicolas. I'm happy to check a PR for such change. The question is whether we get the changes done and tested by the 05.02.22 as thats the date for the RC1 release.

Do you think thats possible?

@PhilETaylor do you plan to do a PR with the changes you proposed that could be merge ready by the 05.02.22?

Im not happy providing the PR for this at short notice sorry.

You can take the Joomla 4.1 code below as a standard alone function, add it to the bottom of restore.php and then sprinkle calls to clearFileInOPCache($file);throughout the restore.php until you are happy (remembering to do it BEFORE each unlink and AFTER each copy/write of a file)

/**
 * Invalidate a file in OPcache.
 *
 * Only applies if the file has a .php extension.
 *
 * @param   string  $file  The filepath to clear from OPcache
 *
 * @return  boolean
 *
 * @since   __DEPLOY_VERSION__
 */
function clearFileInOPCache(string $file): bool
{
	static $hasOpCache = null;

	if (is_null($hasOpCache))
	{
		$hasOpCache = ini_get('opcache.enable')
			&& function_exists('opcache_invalidate')
			&& (!ini_get('opcache.restrict_api') || stripos(realpath($_SERVER['SCRIPT_FILENAME']), ini_get('opcache.restrict_api')) === 0);
	}

	if ($hasOpCache && (strtolower(substr($file, -4)) === '.php'))
	{
		return opcache_invalidate($file, true);
	}

	return false;
}

You can take the Joomla 4.1 code below as a standard alone function, add it to the bottom of restore.php and then

@PhilETaylor I'm also quite busy atm so unless someone brave enough to edit the restore.php volunteers I guess this PR could be the easy way out for the moment. BTW this is not my solution I just did the PR here. Credit goes to @nikosdion for the solution and @richard67 for discovering the bug in the first place. I'm the messenger here...

@zero-24 @richard67 The way I would handle this for Joomla 3 is two pronged.

One, merge #32918 because there is no guarantee that a future Joomla 4 version's script.php won't do something filesystem related in the final finalisation step where we most definitely do not want OPcache to kick in on penalty of site death.

Two, use restore.php from https://github.com/joomla/joomla-cms/blob/4.0.2/administrator/components/com_joomlaupdate/restore.php in Joomla 3.10. This is the Joomla 4 enhanced version of that file which does clear OPcache.

@nikosdion We all know how critical this file for the core updates is. Do you think you can prepare a PR that includes the changes you mentiond? For 3.10.6 the focus would be to do everything that has to be done to get 3.10 to 4.1 updates working. The change for future versions can go with more time into 3.10.7 or any later version.

When its not possible to get such patch together my understanding is that this PR while it is just a workaround can go in and can later be reverted out when the better PHP code fix is there.

@zero-24 I am about to make a PR now. Caveat: I do not have PHP 5.x, 7.0 and 7.1 on any of my dev or test servers. I will need help testing on PHP 5.3, 7.0, 7.4 and 8.0. Let me draft the PR so we can get rolling with the testing.

@zero-24 Also correct, this PR is a good temporary workaround until we can finish testing a better solution.

@zero-24 I am about to make a PR now. Caveat: I do not have PHP 5.x, 7.0 and 7.1 on any of my dev or test servers. I will need help testing on PHP 5.3, 7.0, 7.4 and 8.0. Let me draft the PR so we can get rolling with the testing.

Thank you ? I dont have opcache but i can test older PHP versions still work. Hopefully also @richard67 can help with testing of opcache as well as @dgrammatiko :)

I can help with testing, but not today.

Will close here for now as we have the more complete PR from Nicholas at #36878

@zero-24 fwiw I have tested also this PR and it eradicated the reported issue

I have tested this item ✅ successfully on 73719be

I could reproduce the issue not with every update attempt on my environment with opcache, but very likely when testing several updates in sequence, i.e. updated 3.10-dev to latest 4.1 nightly, then all to the previous state and doing that again (in case of testing this PR with applying the patch before each update).

I have tried now everything to reproduce the issue with the patch applied, but I was not able to reproduce it, so it seems that the patch helps.

@dgrammatiko I've tested today @nikosdion 's PR #37003 , and it seems it finally solves the issue, so this PR here would not be needed anymore. But I suggest that if you close it, don't delete the branch in case if it turns out later that I was wrong again.