SqlBagOStuff uses a SELECT FOR UPDATE followed by a DELETE followed by an INSERT. This seems to be deliberate to ensure atomicity, see r55079. However, the following race condition is possible:
- Process 1 calls incr()
- Process 2 calls incr()
- Process 1 does the SELECT FOR UPDATE
- Process 2 does the SELECT FOR UPDATE
- Process 1 DELETEs the row
- Process 2 tries to DELETE the row. The DELETE fails silently (0 rows affected)
- Process 1 INSERTs the new row
- Process 2 tries to INSERT the new row but dies with a duplicate key error
Because incr() bails if the SELECT FOR UPDATE doesn't return a row, this is a somewhat unlikely race condition, but I've seen it happen several times on my test install when hard refreshing, because each load.php instance calls incr( 'requests-with-session' ) or something similar.
A similar bug in SqlBagOStuff::set() used to exist, see bug 24425 and r70203.
(Note that UPDATE with value=value+1 can't be used because the value is a serialized blob rather than a proper integer.)
This is a really nasty bug that'll cause intermittent RL bugs on SQL-cache-based installs, so I'm making this a blocker for the 1.17 release.
Version: unspecified
Severity: critical