NineChime forum

Furry stuff, oekaki stuff, and other stuff.

You are not logged in.

#1 11-20-2007 14:27:22

Peter
New member

Race condition fix - Removal of get_new_picnumber()

If two users submit a new picture at the same time, one will get the picture ID and the other won't.

get_new_picnumber() is sub-optimal at best, so I've removed it in favour of using the ID_2 for PIC_ID and using mysql_insert_id() to retrieve. Currently I set PIC_ID as backwards compatibility. ID_2 is an auto_increment so it is always guaranteed to be unique by MySQL.

I don't know how this affects existing installations, but I've removed the "NOT NULL" definition on PIC_ID for future installations (install.php).

Diff attached below. Based on 1.3.10


Code:

diff -rNu wacpoteto-1.3.10/oekaki/functions.php devel/oekaki/functions.php
--- wacpoteto-1.3.10/oekaki/functions.php    2007-07-29 17:13:36.000000000 -0700
+++ devel/oekaki/functions.php    2007-11-20 12:05:13.000000000 -0800
@@ -968,36 +968,25 @@
         report_err('Picture is an unsupported filetype.');
     }
 
-
-    // Pic checks out, so reserve a slot
-    $resno = get_new_picnumber();
-    if ($resno == 0) {
-        report_err(__FILE__.': Cannot allocate new PIC_ID for upload.');
-    }
-
-
     // Cleanup slots
     $result = clean_picture_slots();
 
     // Insert new slot
-    $tries = 5;
-    while ($tries > 0) {
-        $result = mysql_query ("INSERT INTO {$OekakiPoteto_Prefix}oekakidta SET usrname='{$upload_name}', postdate=NOW(), comment='$comment', hostname='{$hostname}', PIC_ID='{$resno}', IP='{$address}', title='{$title}', adult='{$adult}', datatype='{$datatype}', edittime='{$time_invested}', animation='{$have_animation}', postlock='1'");
+    $sqlres = "INSERT INTO {$OekakiPoteto_Prefix}oekakidta SET usrname='{$upload_name}', postdate=NOW(), comment='$comment', hostname='{$hostname}', PIC_ID='', IP='{$address}', title='{$title}', adult='{$adult}', datatype='{$datatype}', edittime='{$time_invested}', animation='{$have_animation}', postlock='1'";
 
-        if ($result) {
-            $tries = 0;
-        } else {
-            $tries--;
-            sleep (1);
-        }
-    }
-    if (!$result) {
+    if ( !mysql_query ($sqlres) ) {
         w_error_log ('Upload failed: Picture='.$p_prefix.$resno.'.'.$type.'; Owner='.$OekakiU.'; DB: '.mysql_error());
         report_err('Could not insert picture into database.');
     }
 
+    // Get picture ID
+    $resno = mysql_insert_id();
+
+    // Insert OK, set PIC_ID (backwards compatibility.. lazyness, etc.) -peter@xenophase.net
+    mysql_query ("UPDATE {$OekakiPoteto_Prefix}oekakidta SET PIC_ID = '$resno' WHERE ID_2 = '$resno'");
+
     // Insert OK.  Add member piccount
-    $result = mysql_query ("UPDATE {$OekakiPoteto_MemberPrefix}oekaki SET piccount=(piccount + 1) WHERE usrname='{$OekakiU}'");
+    mysql_query ("UPDATE {$OekakiPoteto_MemberPrefix}oekaki SET piccount=(piccount + 1) WHERE usrname='{$OekakiU}'");
 
 
     // Write file(s)
diff -rNu wacpoteto-1.3.10/oekaki/getoekakibbs.php devel/oekaki/getoekakibbs.php
--- wacpoteto-1.3.10/oekaki/getoekakibbs.php    2007-07-21 21:42:20.000000000 -0700
+++ devel/oekaki/getoekakibbs.php    2007-11-20 12:05:17.000000000 -0800
@@ -115,39 +115,25 @@
 
 // Insert new slot into database
 if ($mode == 'norm' || $mode == 'anim') {
-    $resno = get_new_picnumber();
-    if ($resno == 0) {
-        w_error_log (__FILE__.": Cannot allocate new PIC_ID.  User: $OekakiU");
-        quiet_exit($langop_err_picts);
-    }
-
     // Cleanup slots
     $result = clean_picture_slots();
 
+    $animation_type = ( $mode == 'anim' ) ? 1 : 0;
     $thetime = time() - $edittimes;
 
     // Insert new slot
-    $tries = 5;
-    while ($tries > 0) {
-        $sqlres = "INSERT INTO {$OekakiPoteto_Prefix}oekakidta SET usrname='$OekakiU', postdate=NOW(), hostname='$hostname', comment='', PIC_ID='$resno', IP='$address', datatype=1, edittime='$thetime', password=''";
-        if ($mode == 'anim') {
-            $sqlres .= ', animation=1';
-        } else {
-            $sqlres .= ', animation=0';
-        }
-        $result = mysql_query ($sqlres);
+    $sqlres = "INSERT INTO {$OekakiPoteto_Prefix}oekakidta SET usrname='$OekakiU', postdate=NOW(), hostname='$hostname', comment='', PIC_ID='', IP='$address', datatype=1, edittime='$thetime', password='', animation = '$animation_type'";
 
-        if ($result) {
-            $tries = 0;
-        } else {
-            $tries--;
-            sleep (1);
-        }
-    }
-    if (!$result) {
+    if ( !mysql_query ($sqlres) )    {
         w_error_log (__FILE__.": DB cannot save image.  User: $OekakiU, Number: $resno, SQL: ".mysql_error());
         quiet_exit($langop_err_saveimg);
     }
+
+    // Get picture ID
+    $resno = mysql_insert_id();
+
+    // Insert OK, set PIC_ID (backwards compatibility.. lazyness, etc.) -peter@xenophase.net
+    mysql_query ("UPDATE {$OekakiPoteto_Prefix}oekakidta SET PIC_ID = '$resno' WHERE ID_2 = '$resno'");
 }
 
 
diff -rNu wacpoteto-1.3.10/oekaki/install.php devel/oekaki/install.php
--- wacpoteto-1.3.10/oekaki/install.php    2007-09-24 00:07:36.000000000 -0700
+++ devel/oekaki/install.php    2007-11-20 12:14:52.000000000 -0800
@@ -632,7 +632,7 @@
 
         {
             // OekakiData v1.0.0
-            $sql = "CREATE TABLE IF NOT EXISTS `{$OekakiPoteto_Prefix}oekakidta` (ID_2 int(11) NOT NULL auto_increment, PIC_ID int(11) NOT NULL default 0, usrname varchar(255) NOT NULL default '', comment text, postdate datetime NOT NULL default '0000-00-00 00:00:00', hostname text, email varchar(255) default NULL, url text, IP text, title varchar(255) default NULL, lastcmt datetime NOT NULL default '0000-00-00 00:00:00', adult tinyint(2) NOT NULL default '0', edittime int(20) unsigned default NULL, animation tinyint(2) default '0', datatype tinyint(2) default '0', archive tinyint(2) NOT NULL default '0', postlock tinyint(4) NOT NULL default '0', ";
+            $sql = "CREATE TABLE IF NOT EXISTS `{$OekakiPoteto_Prefix}oekakidta` (ID_2 int(11) NOT NULL auto_increment, PIC_ID int(11) default 0, usrname varchar(255) NOT NULL default '', comment text, postdate datetime NOT NULL default '0000-00-00 00:00:00', hostname text, email varchar(255) default NULL, url text, IP text, title varchar(255) default NULL, lastcmt datetime NOT NULL default '0000-00-00 00:00:00', adult tinyint(2) NOT NULL default '0', edittime int(20) unsigned default NULL, animation tinyint(2) default '0', datatype tinyint(2) default '0', archive tinyint(2) NOT NULL default '0', postlock tinyint(4) NOT NULL default '0', ";
 
             // OekakiData v1.1.0
             $sql .= "threadlock TINYINT NOT NULL DEFAULT 0, px MEDIUMINT DEFAULT 0, py MEDIUMINT DEFAULT 0, ptype VARCHAR(4), ttype VARCHAR(4), rtype VARCHAR(4), usethumb TINYINT NOT NULL DEFAULT 0, ";
diff -rNu wacpoteto-1.3.10/oekaki/paintbbsget.php devel/oekaki/paintbbsget.php
--- wacpoteto-1.3.10/oekaki/paintbbsget.php    2007-06-19 05:54:32.000000000 -0700
+++ devel/oekaki/paintbbsget.php    2007-11-20 12:06:09.000000000 -0800
@@ -127,39 +127,25 @@
 
 // Insert new slot into database
 if ($mode == 'norm' || $mode == 'anim') {
-    $resno = get_new_picnumber();
-    if ($resno == 0) {
-        w_error_log (__FILE__.": Cannot allocate new PIC_ID.  User: $OekakiU");
-        quiet_exit($langop_err_picts);
-    }
-
     // Cleanup slots
     $result = clean_picture_slots();
 
+    $animation_type = ( $mode == 'anim' ) ? 1 : 0;
     $thetime = time() - $edittimes;
 
     // Insert new slot
-    $tries = 5;
-    while ($tries > 0) {
-        $sqlres = "INSERT INTO {$OekakiPoteto_Prefix}oekakidta SET usrname='$OekakiU', postdate=NOW(), hostname='$hostname', comment='', PIC_ID='$resno', IP='$address', datatype=0, edittime='$thetime', password=''";
-        if ($mode == 'anim') {
-            $sqlres .= ', animation=1';
-        } else {
-            $sqlres .= ', animation=0';
-        }
-        $result = mysql_query ($sqlres);
+    $sqlres = "INSERT INTO {$OekakiPoteto_Prefix}oekakidta SET usrname='$OekakiU', postdate=NOW(), hostname='$hostname', comment='', PIC_ID='', IP='$address', datatype=1, edittime='$thetime', password='', animation = '$animation_type'";
 
-        if ($result) {
-            $tries = 0;
-        } else {
-            $tries--;
-            sleep (1);
-        }
-    }
-    if (!$result) {
+    if ( !mysql_query ($sqlres) )    {
         w_error_log (__FILE__.": DB cannot save image.  User: $OekakiU, Number: $resno, SQL: ".mysql_error());
         quiet_exit($langop_err_saveimg);
     }
+
+    // Get picture ID
+    $resno = mysql_insert_id();
+
+    // Insert OK, set PIC_ID (backwards compatibility.. lazyness, etc.) -peter@xenophase.net
+    mysql_query ("UPDATE {$OekakiPoteto_Prefix}oekakidta SET PIC_ID = '$resno' WHERE ID_2 = '$resno'");
 }
 
 
diff -rNu wacpoteto-1.3.10/oekaki/paintsave.php devel/oekaki/paintsave.php
--- wacpoteto-1.3.10/oekaki/paintsave.php    2007-07-19 03:38:42.000000000 -0700
+++ devel/oekaki/paintsave.php    2007-11-20 12:00:52.000000000 -0800
@@ -41,19 +41,6 @@
     }
 }
 
-
-function get_new_picnumber() {
-    // Need DB
-    global $dbconn, $OekakiPoteto_Prefix;
-
-    // Get piccount
-    $result = mysql_query ("UPDATE {$OekakiPoteto_Prefix}oekakimisc SET miscvalue=(miscvalue + 1) WHERE miscname='piccount'");
-    $result = mysql_query ("SELECT miscvalue FROM {$OekakiPoteto_Prefix}oekakimisc WHERE miscname='piccount'");
-
-    return mysql_result ($result, 0);
-}
-
-
 function kill_post_files($resno) {
     // Need DB, names
     global $dbconn, $OekakiPoteto_Prefix;
diff -rNu wacpoteto-1.3.10/oekaki/shiget.php devel/oekaki/shiget.php
--- wacpoteto-1.3.10/oekaki/shiget.php    2007-06-19 05:54:32.000000000 -0700
+++ devel/oekaki/shiget.php    2007-11-20 12:06:24.000000000 -0800
@@ -132,39 +132,25 @@
 
 // Insert new slot into database
 if ($mode == 'norm' || $mode == 'anim') {
-    $resno = get_new_picnumber();
-    if ($resno == 0) {
-        w_error_log (__FILE__.": Cannot allocate new PIC_ID.  User: $OekakiU");
-        quiet_exit($langop_err_picts);
-    }
-
     // Cleanup slots
     $result = clean_picture_slots();
 
+    $animation_type = ( $mode == 'anim' ) ? 1 : 0;
     $thetime = time() - $edittimes;
 
     // Insert new slot
-    $tries = 5;
-    while ($tries > 0) {
-        $sqlres = "INSERT INTO {$OekakiPoteto_Prefix}oekakidta SET usrname='$OekakiU', postdate=NOW(), hostname='$hostname', comment='', PIC_ID='$resno', IP='$address', datatype=$datatype, edittime='$thetime', password=''";
-        if ($mode == 'anim') {
-            $sqlres .= ', animation=1';
-        } else {
-            $sqlres .= ', animation=0';
-        }
-        $result = mysql_query ($sqlres);
+    $sqlres = "INSERT INTO {$OekakiPoteto_Prefix}oekakidta SET usrname='$OekakiU', postdate=NOW(), hostname='$hostname', comment='', PIC_ID='', IP='$address', datatype=1, edittime='$thetime', password='', animation = '$animation_type'";
 
-        if ($result) {
-            $tries = 0;
-        } else {
-            $tries--;
-            sleep (1);
-        }
-    }
-    if (!$result) {
+    if ( !mysql_query ($sqlres) )    {
         w_error_log (__FILE__.": DB cannot save image.  User: $OekakiU, Number: $resno, SQL: ".mysql_error());
         quiet_exit($langop_err_saveimg);
     }
+
+    // Get picture ID
+    $resno = mysql_insert_id();
+
+    // Insert OK, set PIC_ID (backwards compatibility.. lazyness, etc.) -peter@xenophase.net
+    mysql_query ("UPDATE {$OekakiPoteto_Prefix}oekakidta SET PIC_ID = '$resno' WHERE ID_2 = '$resno'");
 }

Last edited by Peter (11-20-2007 14:27:42)

Offline

#2 11-22-2007 00:55:39

Waccoon
Administrator

Re: Race condition fix - Removal of get_new_picnumber()

Hey, cool.  I wasn't aware that PHP had a function for that -- I thought I'd have to do it all manually.

"PIC_ID" and "piccount" need to be deprecated, as they are leftovers from the old sorting system.  Everything should eventually be based on ID_2.  The code you've given me needs to be expanded, but it's good that you've brought this to my attention.

I don't know how this affects existing installations, but I've removed the "NOT NULL" definition on PIC_ID for future installations (install.php).

For some reason, almost everything in OekakiPoteto had "NOT NULL" declared.  Some installations of MySQL will complain if a NULL value is possible when a DEFAULT value is defined, but otherwise it doesn't make a difference whether "NOT NULL" is there or not.

Thanks again.

Offline

#3 11-22-2007 00:58:35

Peter
New member

Re: Race condition fix - Removal of get_new_picnumber()

This is a temporary fix, if you're interested in actually merging these changes into the base I'd be willing to write a complete patch.

If you're interested in discussing pls contact me on MSN/Jabber(Gtalk) peter at wingless dot org

Offline

Board footer

Yep, still running PunBB
© Copyright 2002–2008 PunBB