diff --git a/core/includes/config.inc b/core/includes/config.inc index 4a3a1f7..2e37f9b 100644 --- a/core/includes/config.inc +++ b/core/includes/config.inc @@ -99,13 +99,12 @@ function config_sync() { } if (!lock_acquire(__FUNCTION__)) { - // Another request is synchronizing configuration. Wait for it to complete, - // then return a negative result for UI purposes. We do not make a - // difference between an actual synchronization error and a failed lock, - // because a concurrent request synchronizing configuration is an edge-case - // in the first place and would mean that more than one developer or - // site builder attempts to do it without coordinating with others. - lock_wait(__FUNCTION__); + // Another request is synchronizing configuration. + // Return a negative result for UI purposes. We do not make a difference + // between an actual synchronization error and a failed lock, because a + // concurrent request synchronizing configuration is an edge-case in the + // first place and would mean that more than one developer or site builder + // attempts to do it without coordinating with others. return FALSE; } @@ -155,29 +154,23 @@ function config_sync_save_changes(array $config_changes) { * An array of changes to be loaded. */ function config_sync_invoke_sync_hooks(array $config_changes) { - // Keep a copy of the changes so that modules cannot modify the values by - // taking the array by reference. - $config_changes_copy = $config_changes; - - // @todo Lock writes to all storages to prevent other/unintended config + // @todo Lock writes to all stores to prevent other/unintended config // changes from happening during the import. $target_storage = new DrupalConfig(new DatabaseStorage(NULL)); $source_storage = new DrupalConfig(new FileStorage(NULL)); // Allow all modules to deny configuration changes. - foreach (module_implements('config_sync_validate') as $module) { - $config_changes = $config_changes_copy; - $function = $module . '_config_sync_validate'; - $function($config_changes, $target_storage, $source_storage); - } + // Note: module_invoke_all() can only be used as long as it does not allow + // implementations to take $config_changes by reference, since they are not + // supposed to change the configuration to be imported. + module_invoke_all('config_sync_validate', $config_changes); // Allow modules to react to the configuration changes. - foreach (module_implements('config_sync') as $module) { - $config_changes = $config_changes_copy; - $function = $module . '_config_sync'; - $function($config_changes, $target_storage, $source_storage); - } + // Note: module_invoke_all() can only be used as long as it does not allow + // implementations to take $config_changes by reference, since they are not + // supposed to change the configuration to be imported. + module_invoke_all('config_sync', $config_changes); } /** @@ -201,6 +194,7 @@ function config_sync_invoke_sync_error_hooks(array $config_changes) { $function($config_changes, $target_storage, $source_storage); } catch (ConfigException $e) { + watchdog_exception('config_sync', $e); // Just keep going, because we need to allow all modules to react even if // some of them are behaving badly. } diff --git a/core/modules/config/config.admin.inc b/core/modules/config/config.admin.inc index f63dcd3..2366c5a 100644 --- a/core/modules/config/config.admin.inc +++ b/core/modules/config/config.admin.inc @@ -52,6 +52,10 @@ function config_admin_import_form_submit($form, &$form_state) { drupal_set_message(t('The configuration was imported successfully.')); } else { + // Another request may be synchronizing configuration already. Wait for it + // to complete before returning the error, so already synchronized changes + // do not appear again. + lock_wait(__FUNCTION__); drupal_set_message(t('The import failed due to an error. Any errors have been logged.'), 'error'); } }