Extension downloader does not flush correctly, leading to flaky+circumstantial behavior
Overview
The Extension
system provides a mechanism for downloading upgrades (which is primarily used through the web UI, but it can also run via API/CLI). There is a problem in how file-loading and system-flushing work with the downloader.
I believe this problem has existed since ~v4.2 and has been reported anecdotally. However, the symptoms present in diverse ways (often asymptomatic; occasionally as hard-crashes; occasionally as forgetable quirks), and the symptoms clear up upon inspection. This report is (hopefully) a more precise and reproducible demonstration.
Reproduction steps
I've posted an extension (badflush
) with two version (1.0
, 2.0
) to demonstrate the problem. Both versions implement a hook (but with different logic). To see the problem, you can simply install each version. (This is normally done via web UI, but it's easier to control with API-calls.)
-
Install
badflush-1.0
.cv api Extension.download key=badflush url='http://think.hm/tmp/badflush-1.0.zip'
which has the code:
function badflush_civicrm_managed(&$entities) { $entities[] = [ 'module' => 'badflush', 'name' => 'oldthing', 'entity' => 'OptionValue', 'params' => [ 'version' => 4, 'values' => [ 'option_group_id.name' => 'activity_type', 'label' => 'Old Thing', 'name' => 'oldthing', ], ] ]; }
-
In web UI, open the "New Activity" screen. Look for an activity type named "Old Thing".
-
Install
badflush-2.0
cv api Extension.download key=badflush url='http://think.hm/tmp/badflush-2.0.zip'
This also implements the same hook, but the code is different. "Old Thing" is replaced with "New Thing".
function badflush_civicrm_managed(&$entities) { $entities[] = [ 'module' => 'badflush', 'name' => 'newthing', 'entity' => 'OptionValue', 'params' => [ 'version' => 4, 'values' => [ 'option_group_id.name' => 'activity_type', 'label' => 'New Thing', 'name' => 'newthing', ], ] ]; }
-
In web UI, open the "New Activity" screen. Look for an activity type named "New Thing".
Current behaviour
At step 4, the "New Thing" is not displayed. Instead, it still displays the "Old Thing".
However, if you make a new request to flush caches, then it displays "New Thing".
Expected behaviour
At step 4, the "New Thing" should be displayed.
Comments
The root problem can be understood with this pseudocode for the download/upgrade process:
<?php
/*1:*/ start_civicrm();
/*2:*/ download_and_extract($codeUrl, $localFolder);
/*3:*/ system_flush();
Step 1 loads the old version of badflush.php
. Steps 2 replaces badflush.php
with the new version. Step 3 resets the system. But PHP still has an old copy of badflush.php
in active memory. It doesn't matter how many times you run system_flush()
in this context - it will continue to reset with the old code.
The scope of impact makes it difficult to reproduce.
- The problem only affects people who use the built-in downloader to upgrade code. (Many developers+triagers don't.)
- There is no problem if you install new/clean extensions. (Most developers+triagers rely on feeds that only give the latest version of the extension.)
- There is no problem if you use
git
,svn
,wget
,composer
, or other download tools. - As soon as you do a system-flush, the problem goes away.
- The problem only matters for files+functions which are implicated in actively rehydrating data. Thus, you might notice it with
hook_managed
,hook_alterMenu
,hook_xmlMenu
,hook_container
, etc. But you would not notice it withhook_pre
,hook_post
,hook_alterMailParams
,hook_buildForm
, etc. - If the change affects the main
mymodule.php
, then it's always problematic. If the changes are spread among other files (eg*.mgd.php
files), then... it depends on (exactly) when+how the files are loaded. New files, deleted files, and not-yet-loaded files might work alright - but you cannot reload a PHP file that was loaded before the download. - Even if two systems have the same extension and upgrade to the same version, they may not produce the same errors... if they started from different versions.
I'm not really certain the best way to fix it, but... brainstorming a bit...
- If one changed the system-flush to strictly/only delete old data (never rehydrate data in the same pageload), that could fix it.
- If one could split the upgrade process into multiple PHP requests, that could fix it.
- If you radically change the file-format of the extension (eg use fewer top-level functions and top-level classes; use more data-files and anonymous callables), then you could theoretically resolve it.