(dev/core#4055) ClassLoader - Use separate cache IDs for different configurations of modules

[totten]

Overview

Possible alternative to #25235. It aims to address https://lab.civicrm.org/dev/core/-/issues/4055, although I don't know that my local system is suitable for testing.

Before

When you enable or disable an extension:

1. It deletes the CachedExtLoader.{$envId}.php, then
2. It creates a new version of CachedExtLoader.{$envId}.php, and finally
3. It loads the new version of CachedExtLoader.{$envId}.php.

But in some environments, step (3) is reported to read stale data because the opcode cache has momentarily retained the old data.

After

When you enable or disable an extension, it changes the {$envId} used for the filename CachedExtLoader.{$envId}.php.

Since the {$envId} is different, it doesn't matter whether the opcode cache retains the old data -- because it's retained under the old name. The new name should be clean.

Comments

I haven't specifically tried to reproduce the scenario in 4055. Putting this up to see if the test-suite likes it.

ping @kainuk

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

Note that in terms of the tangential issue I hit - ie totten/civix#287 - that getActiveModuleFiles is loading from the possibly-last-build-relevant Redis - I'm trying to figure out the impact of this on that issue. I suspect it would be unchanged

[totten]

Off the top of my head -- I wouldn't expect an effect one way or the other.

In theory, if all the Redis caches had a similar prefix (eg dmaster/{$envAndModuleId}/{$cacheGroup}/{$cacheKey}), then that might mitigate the issue. But for the use-case of civibuild install or civibuild reinstall, I might lean toward updating the installer -- it should pre-emptively clear caches to ensure that the new install starts from a clean slate.

[kainuk]

Hi @totten, I just tested this PR on my local machine, and solves the problem. The advantage of your solution that is does not change cache mechanism, but just targets the stale problem.

[eileenmcnaughton]

MOP based on @kainuk's testing

test this please

[totten]

Thanks @kainuk. Glad it works.