From ef935da5f239af1b820ad5238505442b776185da Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Tue, 28 Jun 2022 18:27:01 -0700
Subject: [PATCH] (dev/core#3660) CRM_Extension_ClassLoader - Defend against
redundant refreshes
Overview
--------
This is a follow-up to #23824 (c24dd7db7e1e91120fd7daeb7e151f856d6b78c3) which addresses a
regressive edge-case.
Steps to Reproduce
------------------
* Write an extension like `wmf-civicrm` which (a) calls `System.flush` (`rebuildMenuAndCaches()`)
during `hook_install` -- and then (b) loads some class from the same extension.
```php
function foo_civicrm_install() {
civicrm_api3('System', 'flush', []);
CRM_Foo_Helper::doStuff();
}
```
* Try to install the extension.
Before
------
Crashes on loading the class `CRM_Foo_Helper`
After
-----
Loads the class `CRM_Foo_Helper`.
Comments
--------
(1) To see what's happening, consider `CRM_Extension_Manager_Module::onPreInstall()`.
This registers the new classloader and then fires `hook_install` which eventually
fires `rebuildMenuAndCaches()`. With c24dd7db, this resets the classloader again.
But the extension isn't fully installed yet - so it forgets about the new extension.
(2) Is it safe to have some (temporarily) sticky items? Ish. You might
say: "Ah, but what if we need to remove an extension? Won't this static
variable retain stale things?" Doesn't matter. In PHP, classloading is a
one-way-street. (You cannot unload.) So you'll still have old classes in
memory, regardless of whether the `ClassLoader` has some old metadata
about where to find classes.
(3) I'm on the fence about whether this patch is a good idea. Calling
`System.flush` explicitly in this context seems like an invitation to
trouble. OTOH, it worked before #23824, so it can be called a regression.
---
CRM/Extension/ClassLoader.php | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/CRM/Extension/ClassLoader.php b/CRM/Extension/ClassLoader.php
index a46580651d2..bdd9ce6c68e 100644
--- a/CRM/Extension/ClassLoader.php
+++ b/CRM/Extension/ClassLoader.php
@@ -21,6 +21,14 @@ class CRM_Extension_ClassLoader {
*/
const FEATURES = ',psr0,psr4,';
+ /**
+ * A list of recently-activated extensions. This list is retained
+ * even if some ill-advised part of the installer does a `ClassLoader::refresh()`.
+ *
+ * @var array
+ */
+ private static $newExtensions = [];
+
/**
* @var CRM_Extension_Mapper
*/
@@ -97,6 +105,9 @@ class CRM_Extension_ClassLoader {
}
self::loadExtension($loader, $this->mapper->keyToInfo($key), $this->mapper->keyToBasePath($key));
}
+ foreach (static::$newExtensions as $record) {
+ static::loadExtension($loader, $record[0], $record[1]);
+ }
return $loader;
}
@@ -130,6 +141,7 @@ class CRM_Extension_ClassLoader {
if (file_exists($file)) {
unlink($file);
}
+ static::$newExtensions[] = [$info, $path];
if ($this->loader) {
self::loadExtension($this->loader, $info, $path);
}
--
GitLab