In D7, this code:

<?php
var_dump
(lock_acquire('x'));
var_dump(lock_acquire('x'));
?>

returns
bool(true)
bool(true)
.

In D6 it returns

bool(true)
bool(false)

This is due to a bug in the backport.

Files: 
CommentFileSizeAuthor
#9 lock_acquire-2.patch697 bytesc960657
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#7 lock_acquire-1.patch1.25 KBc960657
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch lock_acquire-1.patch. See the log in the details link for more information.
[ View ]
lock_acquire-D6.patch1.25 KBc960657

Comments

Assigned:Unassigned» c960657

Status:Needs review» Reviewed & tested by the community

Ran into this issue as well. The patch corrects the problem.

Status:Reviewed & tested by the community» Needs review

Please provide a more detailed explanation of what is going on and more reviewers if possible. Thanks!

The problem is that the return value from db_query("UPDATE ... WHERE ...") is passed on to db_result() in order to see if any rows matched the WHERE clause of the query.

This is wrong. db_query("UPDATE ...") always returns TRUE, unless an error occurred (note that this is not described in the documentation of db_query()).

The proper way to see if any rows where matched by the WHERE clause of an UPDATE query is to use db_affected_rows(). This is what the patch does. If no rows where affected, it means that our lock is no longer in the database table and thus cannot be extended.

Seeing the same thing here when we try to extend an unexpired lock. As c960657 said, It is not proper to use db_result() on a db_query() return value when an UPDATE query is used. For MySQL systems, the result of mysql_query() where an UPDATE query is issued is simply TRUE or FALSE, not a resource as db_result() expects. FALSE only happens on error, and an UPDATE that doesn't update any rows is not an error, so the db_affected_rows() call in the supplied patch is the proper way of handling this.

From what I can tell, pg_query() returns a result resource regardless of the query type issued, so, if the locking implementation was initially written and tested against Postgres, then that may be why this wasn't caught before. We (and I'm sure most of the people using Drupal), however, use MySQL.

Status:Needs review» Reviewed & tested by the community

We are running this patch in our production environment now without issues.

StatusFileSize
new1.25 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch lock_acquire-1.patch. See the log in the details link for more information.
[ View ]

Renaming patch to force test bot to pick it up.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, lock_acquire-1.patch, failed testing.

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new697 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Reroll (identical patch, just created with Git instead of CVS).

Status:Reviewed & tested by the community» Fixed

Thanks for the extra testing and details, committed and pushed.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.