Java: Making concurrent MySQL queries from multiple clients synchronised

I work at a gaming cybercafe, and we've got a system here (smartlaunch) which keeps track of game licenses. I've written a program which interfaces with this system (actually, with it's backend MySQL database). The program is meant to be run on a client PC and (1) query the database to select an unused license from the pool available, then (2) mark this license as in use by the client PC.

The problem is, I've got a concurrency bug. The program is meant to be launched simultaneously on multiple machines, and when this happens, some machines often try and acquire the same license. I think that this is b开发者_运维知识库ecause steps (1) and (2) are not synchronised, i.e. one program determines that license #5 is available and selects it, but before it can mark #5 as in use another copy of the program on another PC tries to grab that same license.

I've tried to solve this problem by using transactions and table locking, but it doesn't seem to make any difference - Am I doing this right? Here follows the code in question:

    public LicenseKey Acquire() throws SmartLaunchException, SQLException {
    Connection conn = SmartLaunchDB.getConnection();
    int PCID = SmartLaunchDB.getCurrentPCID();

    conn.createStatement().execute("LOCK TABLE `licensekeys` WRITE");

    String sql = "SELECT * FROM `licensekeys` WHERE `InUseByPC` = 0 AND LicenseSetupID = ? ORDER BY `ID` DESC LIMIT 1";
    PreparedStatement statement = conn.prepareStatement(sql);
    statement.setInt(1, this.id);
    ResultSet results = statement.executeQuery();

    if (results.next()) {
        int licenseID = results.getInt("ID");
        sql = "UPDATE `licensekeys` SET `InUseByPC` = ? WHERE `ID` = ?";
        statement = conn.prepareStatement(sql);
        statement.setInt(1, PCID);
        statement.setInt(2, licenseID);
        statement.executeUpdate();
        statement.close();
        conn.commit();
        conn.createStatement().execute("UNLOCK TABLES");
        return new LicenseKey(results.getInt("ID"), this, results.getString("LicenseKey"), results.getInt("LicenseKeyType"));
    } else {
        throw new SmartLaunchException("All licenses of type " + this.name + "are in use");
    }
}

---

You must do two things:

• Wrap your code in a transaction (to avoid autocommit releasing locks immediately)
• Use SELECT ... FOR UPDATE and mysql will give you the lock you need (released on commit)

SELECT ... FOR UPDATE is better than LOCK TABLE as it can possibly get by with row-level locking, instead of automatically locking the whole table

---

According to the online manual, the correct syntax for locking is:

LOCK TABLES ...

and you have

LOCK TABLE ...

but you don't have any error checking. Hence you're probably failing to get the lock and it's silently ignoring that.

FWIW, I'd put your cleanup code (UNLOCK TABLES, conn.commit(), etc) in a finally block to ensure that you always clean up properly in the event of an exception.

As it is, you appear to be potentially leaking database connection handles, and never releasing the lock if there's no free license.

---

I would like to suggest just doing an update statement and checking how many rows where updated. i will write it out in psudo code.

int uniqueId = SmartLaunchDB.getCurrentPCID();;
int updatedRows = execute('UPDATE `licensekeys` SET `InUseByPC` = uniqueId WHERE `InUseByPC` NOT null LIMIT1')
if (updatedRows == 1)
   SUCCESS
else
   FAIL

If it succeeds you can then get the licence key/ID by doing a select.

---

As is so often the case, OP is an idiot. The code I posted was actually working, but I've just discovered a duplicate row in the database - I guess someone entered the same license twice by mistake. This led me to believe that a concurrency bug I had fixed (by introducing table locks) was still unfixed.

Thanks for the general advice, I've introduced better exception handling to this method.