module_implements() cache can be polluted by module_invoke_all() being called (in)directly prior to full bootstrap completion

Original issue report

I can't understand the logic of module_implements() function.

Look on definition:

/** * @param $hook * The name of the hook (e.g. "help" or "menu"). */ ... function module_implements($hook, $sort = FALSE, $refresh = FALSE) { ... if (!isset($implementations[$hook])) { $implementations[$hook] = array(); $list = module_list(FALSE, TRUE, $sort); // second param is $bootstrap = TRUE foreach ($list as $module) { if (module_hook($module, $hook)) { $implementations[$hook][] = $module; } } }

/** ... * @param $bootstrap * Whether to return the reduced set of modules loaded in "bootstrap mode" * for cached pages. See bootstrap.inc. */ ... function module_list($refresh = FALSE, $bootstrap = TRUE, $sort = FALSE, $fixed_list = NULL) { ... if ($bootstrap) { $result = db_query("SELECT name, filename, throttle FROM {system} WHERE type = 'module' AND status = 1 AND bootstrap = 1 ORDER BY weight ASC, filename ASC"); } ...

/** * Define the critical hooks that force modules to always be loaded. */ function bootstrap_hooks() { return array('boot', 'exit'); }

1. I'm calling i.e. module_implements('help') expecting list of all modules that implement that hook
2. module_implements calling module_list with $bootstrap param = TRUE?
$list = module_list(FALSE, TRUE, $sort); // second param is $bootstrap = TRUE
3. So instead of receiving all modules with hook_help, I'm getting modules with this hook only which are registered as $bootstrap only

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Comment over 1 year ago →
🇺🇸United States xjm

Comment about 1 year ago →

🇩🇪Germany Fabianx

While we never got #94 in, it also has other problems.

That said I think we can fix the remaining edge cases, by doing this:

diff --git a/includes/module.inc b/includes/module.inc
index 494924f..24e5dd7 100644
--- a/includes/module.inc
+++ b/includes/module.inc
@@ -833,6 +833,11 @@ function module_hook_info() {
  * @see module_implements()
  */
 function module_implements_write_cache() {
+  // The list of implementations includes vital modules only before full
+  // bootstrap, so do not write cache if we are not fully bootstrapped yet.
+  if (drupal_static("drupal_bootstrap_finished") != TRUE {
+    return;
+  }

and then set the drupal_static at the end of the bootstrap full function.

That way - that at phase is set when it's entered, not when it's completed - would no longer bite us here.

Comment about 1 year ago →
🇨🇦Canada joseph.olstad
#122 makes sense to me. I have not yet tested this however.
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update about 1 year ago
2,161 pass
Comment about 1 year ago →
🇮🇹Italy marco
The attached patch implements #122, with an addendum from @peximo to make sure that calling a full bootstrap more than once does not break the logic.
Comment 9 months ago →
🇮🇹Italy apaderno Brescia, 🇮🇹
Status changed to RTBC 9 months ago7:27pm 11 April 2024
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update 9 months ago
Patch Failed to Apply
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update 9 months ago
Patch Failed to Apply
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update 9 months ago
Patch Failed to Apply
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update 9 months ago
Patch Failed to Apply
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update 9 months ago
Patch Failed to Apply
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update 9 months ago
Patch Failed to Apply
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update 9 months ago
Patch Failed to Apply
Comment 9 months ago →
🇺🇸United States darren oh Lakeland, Florida
Merge request !8831Issue #496170: module_implements() cache can be polluted by module_invoke_all() being called (in)directly prior to full bootstrap completion → (Open) created by apaderno
Pipeline finished with Failed
6 months ago
Total: 307s
#228226

module_implements() cache can be polluted by module_invoke_all() being called (in)directly prior to full bootstrap completion

Problem/Motivation

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Original issue report

Merge Requests

!8831module_implements() cache can be polluted by module_invoke_all() being called (in)directly prior to full bootstrap completion
Open

Comments & Activities

module_implements() cache can be polluted by module_invoke_all() being called (in)directly prior to full bootstrap completion

Problem/Motivation

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Original issue report

Merge Requests

!8831module_implements() cache can be polluted by module_invoke_all() being called (in)directly prior to full bootstrap completionOpen

Comments & Activities

!8831module_implements() cache can be polluted by module_invoke_all() being called (in)directly prior to full bootstrap completion
Open