after about the 20th client of mine did this during training, I decided to file and patch this problem.

The problem:
On a fresh Drupal 7 installation with standard Drupal modules enabled, it is possible to inadvertently disable modules simply by clicking "Save Configuration" twice in succession before the page has had time to reload. I run into this a lot with clients who dont know any better, and don't see the 'loading...' that is being displayed in the overlay. They tend to think nothing happened and click submit again.

The classic symptom is the user asking "where did the toolbar go?" After digging into it, it's almost always that the toolbar, shortcuts and sometimes the overlay modules have been disabled, even though the user never unchecked them. Sometimes even more modules have been disabled - it really depends on how long the user waits between clicks.

This seems to be most common on WAMP or possibly WIMP installs, but also happens on MAMP and other configurations. Generally speaking, as long as the modules page takes a reasonably long amount of time to process and load, this problem becomes more noticeable.

The solution:

Rather than tracking down the race condition that is taking place in the system module build process, it seems that it might make the most sense to implement a lock during the module save process to prevent either users hitting save twice, or two users hitting save at the same time. This seems to do the trick in preventing this problem from reoccurring.

see attached patch for lock implementation.

Hope this helps someone!
-Chris

CommentFileSizeAuthor
module_save_doesnt_lock.patch1018 bytesnetw3rker

Comments

netw3rker’s picture

netw3rker commented
Title: Modules get disabled during module save race condition » Modules get disabled during a " module save" race condition
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Can't we avoid any server-side complexity by simply ensuring that the submit button on the modules page can't be triggered twice? :)

moshe weitzman’s picture

moshe weitzman commented

There is already a lock on modules page submit, but it is specific to menu rebuild. I guess we could do a lock around the whole form submit

damiankloip’s picture

damiankloip commented
Version: 7.x-dev » 8.x-dev

I think the lock plan is the only real way to go here. JS is out I would say. I agree with #3.

The initial patch has the lock in system_modules_confirm_form, which wont always be called? Either way, I think this also has to move into 8.x first.

moshe weitzman’s picture

moshe weitzman commented

I think we should understand the problem a bit more. Exactly why would toolbar become disabled if it was never passed to module_disable().

netw3rker’s picture

netw3rker commented

Yeah i feel JS is out, it really only stops a user with JS enabled from clicking 2x, but not someone clicking refresh, or two people submitting at the same time.

the module save page doesn't have the mentioned locking system on it, moshe is thinking of the menu_rebuild system that has a lock/wait on it to make it single threaded.

per #5, its not just those modules, one users hit it late enough in the game that the views_ui module got disabled! I'm not 100% sure this race condition is something that can fully understood, but my guess (and this is just a guess) is that the system table gets wiped out initially and then begins being populated with enabled modules. when the 2nd process does its form_submit, the values from the db are loaded as available options for the checkboxes, but is missing all of the deleted modules. that submit then wipes the table and populates with just the modules it knows about, leaving a ton of previously enabled modules disabled.

I'll do more digging if it is so demanded, but i think that's probably really close to the crux of the problem. adding a lock/wait system definitely solves this too.

Bojhan’s picture

Bojhan commented

We did think about this already in http://drupal.org/node/483876, I think there we sided against it because of the side-behaviours. Good to get this checked by a11y peeps

mgifford’s picture

mgifford
he/him
Primary language English
commented

@Bojhan, I just looked at the patch. I don't see any UI changes that would affect accessibility. Didn't install it, but guess I need some clarification. I didn't look at #483876: Disable install button once clicked.

damiankloip’s picture

damiankloip commented

I think the outcome so far was that this wouldn't affect the front end. i.e. the button being disabled is not an option.

pwolanin’s picture

pwolanin commented
Status: Needs review » Needs work

You need to acquire the lock before proceeding even after the wait it up.

pwolanin’s picture

pwolanin commented

I'm adding this kind of code right now to a contrib module:

function _queue_runner_lock_acquire($lock, $error_message, $data = array()) {
  if (!lock_acquire($lock)) {
    $retry = 0;
    while (lock_wait($lock) || !lock_acquire($lock)) {
      $retry +=1;
      if ($retry > QUEUERUNNER_LOCK_MAX_RETRIES) {
        throw new QueueRunnerException($error_message, $data);
      }
    }
  }
}

This kind of helper should maybe be in core even.

pwolanin’s picture

pwolanin commented
Issue summary: View changes

fixed some grammer issues

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

catch’s picture

catch
he/him
Primary language English
commented
Version: 8.9.x-dev » 7.x-dev
Issue tags: +Bug Smash Initiative

It's no longer possible to disable modules on this page - since modules can't be disabled. Uninstall happens on a different route. Moving back to 7.x

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.