To keep the internal book-keeping in synch with the reality reflected in the database the lock_may_be_available routine should unset($locks[$name]); before exiting. This is now consistent with the behaviour in lock_acquire when it detects a broken lock.

Comments

damien tournoud’s picture

lock_may_be_available() assumes that the caller doesn't already have the lock (as hinted in the function documentation). That said, it cannot hurt. Care to roll a patch?

lostchord’s picture

Documentation should never make 'hints', that's just a cover for sloppy documentation.

I'll get round to rolling patches when I've finished wandering through core. I'm just picking a few interesting bits at present and the hit rate is a bit alarming.

Edit:

I'm a relative newb (very, in fact) to PHP. My background has a large amount of assembler in it. One thing I'm working through at present, and I'd like to get sorted before 'rolling patches', is the best way to return success/failure AND details of the success/failure. My old-time VAX experience makes me love the 'lower bit set is success, lower bit clear is failure' pattern with the exact value (all the other bits) providing more detail that you can make decisions on. So, for example, I can execute a SELECT query and have it fail and pass that back (it failed) along with additional details like "nothing returned", or "database connection failed", or..... Some standardised way of doing this in all situations would allow far more robust code to be developed without the need to do really detailed research into the specifics of every call being made. In this context documentation is VERY specific about what you can do and what you can expect the results to be. This is a nice example (one of many) in the documentation set here.

My take on this little scan I'm doing at the moment is that 'Core should be absolutely bullet proof'. That means protecting against sloppy coding now. It also means protecting against well meaning Core changes that are done in good faith at some later date that might break todays code. It's basic defensive programming, and 40 years of experience has given me plenty of opportunities to see it pay off big time.

cheers

NROTC_Webmaster’s picture

Version: 7.0-alpha4 » 8.x-dev

I'm not very familiar with this function but my understanding is that you want to return true if it breaks a lock or if the lock is not set. In keeping with this you would not want to unset the lock when returning FALSE. The only way I could come up with a solution for this is the following. Again though since I'm not familiar with this look this function over and let me know if this is what you were looking for. I can't really imagine a scenario where this would be necessary but if it is what you were looking for I can make the patch.

function lock_may_be_available($name) {
  global $locks;

  $lock = db_query('SELECT expire, value FROM {semaphore} WHERE name = :name', array(':name' => $name))->fetchAssoc();
  if (!$lock) {
+    if (isset($locks[$name])) {
+      // The lock was broken.
+      unset($locks[$name]);
+    }
    return TRUE;
  }
  $expire = (float) $lock['expire'];
  $now = microtime(TRUE);
  if ($now > $expire) {
    // We check two conditions to prevent a race condition where another
    // request acquired the lock and set a new expire time. We add a small
    // number to $expire to avoid errors with float to string conversion.
-    return (bool) db_delete('semaphore')
+    $result = (bool) db_delete('semaphore')
      ->condition('name', $name)
      ->condition('value', $lock['value'])
      ->condition('expire', 0.0001 + $expire, '<=')
      ->execute();
+    if ($result && isset($locks[$name]) {
+      // The lock was broken.
+      unset($locks[$name]);
+    }
+    return $result;
  }

  return FALSE;
}
superspring’s picture

Status: Active » Needs review
StatusFileSize
new1.6 KB

As described above.

Status: Needs review » Needs work

The last submitted patch, 4: lock_may_be_available-806708-4.patch, failed testing.

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.

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

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Issue summary: View changes
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative

@lostchord, Thank you for reporting this problem. We rely on issue reports like this one to resolve bugs and improve Drupal core.

There has been no activity here for 10 years. Is this change still desired?

Since we need more information to move forward with this issue, I am setting the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

quietone’s picture

Status: Postponed (maintainer needs more info) » Closed (outdated)

I asked in #bugsmash about this. larowlan responded that this can be closed because it is referring to global $locks which is not used anymore.

Therefor closing as outdated.