Duplicate Short URLs

Well, i am wondering if it's not around here :

In shurly.module

function shurly_shorten()

  elseif (is_null($custom)) {
    $latest = shurly_get_latest_short($long_url, $account->uid);
    if ($latest) {
      $no_save = TRUE;
      $success = TRUE;
      $short = $latest;
    }

In shurly_get_latest_short()

/**
 * Get the latest generated short URL by a given user for a given long URL
 */
function shurly_get_latest_short($long, $uid) {
  return db_query("SELECT source FROM {shurly} WHERE destination = :long AND uid = :uid AND custom = 0 AND active = 1 ORDER BY rid DESC", array('long' => $long, 'uid' => $uid))->fetchField();;
}

It checks if the long url is in the database. But since the $account->uid can be different...
Also in the request, there is an extra semicolon.

Sounds like a race condition, created timestamps are the same

| rid    | uid | source | destination  | created    | count | last_used  | custom | active |
+--------+-----+--------+-------------------------------------------------------------------------------------------+------------+-------+------------+--------+--------+
| 304744 |   1 | Z6zi   | http://XXXXX | 1444251663|     5 | 1444272095 |      0 |      1 |
| 304745 |   1 | Z6zi  | http://yyyy    | 1444251663 |     0 |          0 |      0 |      1 |
+--------+-----+--------+-------------------------------------------------------------------------------------------+------------+-------+------------+--------+--------+

This patch adds locking support to shurly_shorten(). Fixed our problems.

Another solution may to be set an INDEX on the Shurt URL column in the table. then it will error out when trying to create a duplicate.

Just my 2 cents but @RobLoach's suggestion about setting an INDEX on the 'source' column might be less problematic and more performant (at the cost of some additional space in the database). Locks can be problematic in general and lock contention can slow things down...

Can you make a patch for this ?

Hmmm interesting - 'source' is already defined as an index. Makes me wonder how this didn't throw an error. Or, maybe, it did but error messages were hidden?

We can make the 'source' field unique, but that wont solve the race problem, it would just cause an sql error. I dont think that is expected behaviour, hence the locking.

No, it won't 'solve' the race condition, but it will prevent duplicates, and the code could catch the SQL error and regenerate the short url. As I said, the locking will work - but locks have problems of their own (lost locks causing things to block, contention delays ...).