fix: an install rollback drops only the database it created - #926
Merged
Merged
Conversation
MysqlSetup::rollback() ended a failed install with an unconditional DROP DATABASE, and nothing established whose database it was. install/install is unauthenticated by necessity, and Installer::install() calls checkDatabaseAvailability() before anything is created and createDatabase() well after it, with no lock between — so two requests both pass the availability check, the second fails on CREATE SCHEMA because the name is taken, and its rollback dropped the database the first had just finished installing into. config.xml already said installed=1, so the instance claimed to be installed with no schema behind it. Two near-simultaneous requests is all it takes; an impatient double-click on the install button will do it. The same method already drops the runtime user only when $dbUser names one — only when this run created it. The database had no such flag, and now does: Installer sets it the moment createDatabase() returns, and the drop is gated on it. The hazard was understood when this was written — the comment above checkDatabaseAvailability() says a failure there must not trigger a rollback that could touch pre-existing data. It was enforced at the check rather than at the rollback, and only the check is racy. This does not serialise concurrent installs: the loser still fails, correctly, on the schema already existing. It makes losing the race harmless rather than destructive.
blaipr
deleted the
fix/an-install-rollback-drops-only-what-it-created
branch
September 17, 2026 01:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MysqlSetup::rollback()ended a failed install with an unconditionaland nothing established whose database that was.
install/installis unauthenticated by necessity.Installer::install()callscheckDatabaseAvailability()before anything is created andcreateDatabase()well after it, withno lock in between — so two requests both pass the availability check, the second fails on
CREATE SCHEMAbecause the name is now taken, lands in thecatch, and drops the database thefirst had just finished installing into.
config.xmlalready saidinstalled=1, so the instanceclaimed to be installed with no schema behind it and every later request went to
error/databaseError.Two near-simultaneous requests is all it takes. No attacker is required — an impatient double-click
on the install button does it.
The fix is already written, one field over
The same method drops the runtime user only when
$dbUsernames one — that is, only when thisrun created it. The database had no such flag. It does now:
Installersets$createdDatabasethemoment
createDatabase()returns, and the drop is gated on it.The hazard was understood when this was written; the comment above
checkDatabaseAvailability()says so —
— it was enforced at the check rather than at the rollback, and only the check is racy.
What this does and does not fix
It removes the destructive outcome: the losing request now fails and drops nothing, and the winner's
install survives. It does not serialise concurrent installs — the loser still gets an error
about the schema already existing, which is the correct answer to what it asked. A lock across the
whole install would be a separate change; this one makes losing the race harmless.
Hosting mode is untouched: it takes the other branch, dropping the named tables and views rather
than a database, and
createDatabase()is a documented no-op there.Tests
Four, two new and two rewritten:
createDatabase()returns drops no database — the losingrequest's exact shape;
schema and refuse every retry;
MySQLTest's two existing non-hosting rollback tests now saycreatedDatabase: true, since thatis the case they were always describing;
exec()is never reached when the flag is false.Mutation-verified: reverting
src/fails both of the newInstallerTestcases.Worth noting for whoever writes the next test here — the existing rollback tests all set
hostingMode(true), which skipssetupDbUser(). A non-hosting test has to stub it, or thedestructuring at
Installer.php:226raises "Undefined array key 0" and the test passes with awarning rather than failing.