Locking framework ungrokably fails testing [#956948]

In #802446: Cache stampede protection: drupal_render() and page cache (#79 and on) we've found that minor adjustments to lock_wait()/lock_acquire() timeouts cause the test bot to crash and burn with an error like:

FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.

For a concrete example, http://drupal.org/node/802446#comment-3639992 shows that lock_wait($cid, 5) reliably passes, but lock_wait($cid) reliably fails!

This doesn't make one bit of sense. Let's figure it out.

Comment	File	Size	Author
#3	operator_precedence.patch	1.16 KB	carlos8f
#1	lock_test.patch	1.15 KB	carlos8f

Comments

Comment #1

carlos8f commented 29 October 2010 at 15:43

Status:

Active

» Needs review

Status	File	Size
new	lock_test.patch	1.15 KB

In theory, this should fail. I've taken http://drupal.org/node/802446#comment-3530844 and stripped out things that I think don't affect the test failure.

Comment #2

29 October 2010 at 18:00

Status:

Needs review

» Needs work

The last submitted patch, lock_test.patch, failed testing.

Comment #3

carlos8f commented 29 October 2010 at 18:10

Status:

Needs work

» Needs review

Status	File	Size
new	operator_precedence.patch	1.16 KB

Good. Now fixing the operator precedence bug we found.

interdiff lock_test.patch operator_precedence.patch 
diff -u includes/common.inc includes/common.inc
--- includes/common.inc	29 Oct 2010 03:16:31 -0000
+++ includes/common.inc	29 Oct 2010 03:16:31 -0000
@@ -5573,7 +5573,7 @@
   }
   $bin = isset($elements['#cache']['bin']) ? $elements['#cache']['bin'] : 'cache';
 
-  if (!$cache = cache_get($cid, $bin) && !lock_acquire($cid)) {
+  if (!($cache = cache_get($cid, $bin)) && !lock_acquire($cid)) {
     lock_wait($cid);
     if (!$cache = cache_get($cid, $bin)) {
       lock_acquire($cid);

Comment #4

carlos8f commented 29 October 2010 at 19:02

Status:

Needs review

» Fixed

#1 made the bot run around in circles for 2 hours, and now we've demonstrated that it was a simple operator precedence bug. Tracking down the actual loop might be difficult, but at least we have isolated what causes it.