Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Background
Features has a directory /includes/ with PHP files for different modules, e.g. /includes/features.ctools.inc for ctools.
The function features_include() includes all of those files for those modules that are currently enabled.
It has a static variable $once to make sure that it is called only once in a typical request.
The hook implementations features_modules_enabled() and features_modules_disabled() call features_include() with a $reset parameter, to make sure the files for newly enabled modules are also included.
When more than one modules are enabled at once, the hook fires only once in module_enable().
However, some caches are refreshed after the enabling of every individual module.
Some of these cache clears might trigger features_include() without the $reset parameter.
Problem/Motivation
When bulk activating modules (eg by using modules_enable()) the hook_modules_enabled() is only called at the very end after activating all modules. Since other other caches (especially `module_implements()`) are cleared after every module this means features sometimes produces “Call to undefined function …” errors.
Steps to reproduce
I don’t have a minimal set of steps to reproduce at the moment, but enabling a feature that pulls in ctools as dependency on a fresh install seems to be a good candidate.
Proposed resolution
- Use
drupal_static()infeatures_include(). (patch in the comments) - Hope that #1891356: D7: Reset drupal static caches when a module is enabled or disabled. gets resolved at one point.
- Remove the cache flushes in
features_modules_enabled()andfeatures_modules_disabled()as Drupal core takes care of it.
Remaining tasks
- Review the patch
- Get the Drupal core issue fixed.
User interface changes
None
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | features-3176448-drupal-static-9.patch | 468 bytes | torotil |
Comments
Comment #2
torotil commentedComment #3
donquixote commentedQuick question: Is this a regression since the last stable 7.x-2.11, or was it already broken then?
Comment #5
torotil commentedI don’t think it’s a regression, no.
Comment #6
donquixote commentedThe usage of drupal_static() seems a bit funny compared to how other modules use it.
Also, the subsequent comparison should check for FALSE instead of isset().
And we should use === NULL instead of !isset(), if the variable is known to exist.
Overall I don't see how the patch should solve the problem.
It only makes a difference if features_include() is called without the $reset parameter.
But where would this be the case? Which of the calls to features_include() happens after a module is enabled, which does not already have the $reset parameter?
Comment #7
donquixote commentedI am adding a "Background" section to the issue summary, please review.
I think I understand better now.
Comment #8
donquixote commentedIf not, we could abuse another hook, e.g. hook_registry_files_alter(), to trigger a features_include(TRUE).
Comment #9
torotil commentedOk, I was trying to use the static fast pattern. But I don’t think this function is even called often enough to warrant that. Here is a patch with the simple
drupal_static()pattern.Comment #10
torotil commentedYour analysis is spot on. Nice idea to abuse
hook_registry_files_alter()this seems to be called exactly once and early after each module is enabled. The only thing that I’m afraid of is that too many modules start to include such workarounds for #1891356: D7: Reset drupal static caches when a module is enabled or disabled..Comment #11
donquixote commentedI don't mind some static fast, but I had the impression it was not done correctly. Or was I mistaken?
The new patch seems ok.
But I assume this will only do anything after core is fixed, right?
Also we should be aware that with this patch (plus core fix) some operations will run more than once in a request when enabling new modules.
E.g. ctools_features_declare_functions().
I think it is ok, because this function does call function_exists() before it declares new functions. But we should at least have a closer look at this and make sure this won't cause problems.
Comment #12
torotil commented@donquixote A profile install via drush already calls
module_enable()for every single module it enables so I don’t think there will be problems with that.Yes, the condition was done wrongly, but I’d prefer not to use static fast for something that’s not called very often for the typical request anyway.