[dw-free] Fix race condition in the payment system

[mark]

[commit: http://hg.dwscoalition.org/dw-free/rev/d5c172bccac3]

Fix race condition in the payment system

There's a race condition here that can sometimes cause us to mark a payment
as having an internal error when in reality it is just in that magic moment
after Gearman has finished processing it but we haven't registered the
change in the database yet.

This fix adds a second check to check the database again to see if it's been
updated. Also, I have to say, this fix was suggested by Denise, I just
implemented it.

Patch by mark.

Files modified:

• cgi-bin/DW/Shop/Engine/CreditCard.pm

--------------------------------------------------------------------------------
diff -r e434854180fb -r d5c172bccac3 cgi-bin/DW/Shop/Engine/CreditCard.pm
--- a/cgi-bin/DW/Shop/Engine/CreditCard.pm	Fri Dec 30 20:11:04 2011 +0800
+++ b/cgi-bin/DW/Shop/Engine/CreditCard.pm	Sun Jan 01 03:54:12 2012 +0000
@@ -123,18 +123,26 @@
     # if the job is known to the server, that means it's in the queue somewhere, so we are
     # okay to just return whatever state the row has (which should be 'queued')
     return $row if $js && $js->known;
+    return $row unless $row->{jobstate} eq 'queued';
+
+    # if we get here and it says 'queued', that means that we think the job is in Gearman,
+    # but by this point we know it's not. we need to check the database again to see if
+    # the state has been updated. this prevents a race condition we've sometimes seen.
+    my $row2 = $dbh->selectrow_hashref( 'SELECT * FROM cc_trans WHERE cctransid = ?', undef, $cctransid );
+    die "Database error: " . $dbh->errstr . "\n"
+        if $dbh->err;
 
     # now, if the job is not known, and we are still 'queued', something terrible happened
     # like the worker crashed or the gearman server crashed
-    if ( $row->{jobstate} eq 'queued' ) {
-        $row->{jobstate} = 'internal_failure';
-        $row->{joberr} = 'Task no longer known to Gearman.';
+    if ( $row2->{jobstate} eq 'queued' ) {
+        $row2->{jobstate} = 'internal_failure';
+        $row2->{joberr} = 'Task no longer known to Gearman.';
         $dbh->do( 'UPDATE cc_trans SET jobstate = ?, joberr = ?, gctaskref = NULL WHERE cctransid = ?',
-                  undef, $row->{jobstate}, $row->{joberr}, $cctransid );
+                  undef, $row2->{jobstate}, $row2->{joberr}, $cctransid );
         die $dbh->errstr if $dbh->err;
     }
 
-    return $row;
+    return $row2;
 }
 
 # try_capture( ...many values... )
--------------------------------------------------------------------------------

Comments

[denise]

\o/