Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix transaction nesting caused by LocalFile.

Browse source 
The transaction bracket in LocalFile::recordUpload2 used to span a call
to WikiPage::doEdit, which in turn opens a transaction. Nesting transactions
this way does not work: the first transaction is committed prematurely when
the second one starts. This may cause serious database corruption and
generally exciting behavior.

This change commits LocalFile's own transaction before any interaction
with WikiPage. There may be a race condition here, but that case seems to
be already handled in the code. Also, in the previous "broken" state, all
transactional protection got lost anyway - so this should at least be no
worse than what we had.

This may be changed back if and when we have support for nested
transactions.

Change-Id: I20d90fedb2d19c64ccf0f3942ddda756fe511c12
This commit is contained in:
daniel 2012-08-27 14:46:23 +02:00 committed by Aaron
parent b2602ef651
commit 8322771ec3
1 changed files with 20 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
includes/filerepo/file/LocalFile.php
View file

@ -1154,7 +1154,9 @@ class LocalFile extends File {
$logId = $log->addEntry( $action, $descTitle, $comment, array(), $user );
wfProfileIn( __METHOD__ . '-edit' );
if ( $descTitle->exists() ) {
$exists = $descTitle->exists();
if ( $exists ) {
# Create a null revision
$latest = $descTitle->getLatestRevID();
$nullRevision = Revision::newNullRevision(
@ -1169,6 +1171,15 @@ class LocalFile extends File {
wfRunHooks( 'NewRevisionFromEditComplete', array( $wikiPage, $nullRevision, $latest, $user ) );
$wikiPage->updateRevisionOn( $dbw, $nullRevision );
}
}
# Commit the transaction now, in case something goes wrong later
# The most important thing is that files don't get lost, especially archives
# NOTE: once we have support for nested transactions, the commit may be moved
# to after $wikiPage->doEdit has been called.
$dbw->commit( __METHOD__ );
if ( $exists ) {
# Invalidate the cache for the description page
$descTitle->invalidateCache();
$descTitle->purgeSquid();
@ -1178,16 +1189,18 @@ class LocalFile extends File {
# Squid and file cache for the description page are purged by doEdit.
$status = $wikiPage->doEdit( $pageText, $comment, EDIT_NEW | EDIT_SUPPRESS_RC, false, $user );
if ( isset( $status->value['revision'] ) ) {
$dbw->update( 'logging', array( 'log_page' => $status->value['revision']->getPage() ), array( 'log_id' => $logId ), __METHOD__ );
if ( isset( $status->value['revision'] ) ) { // XXX; doEdit() uses a transaction
$dbw->begin();
$dbw->update( 'logging',
array( 'log_page' => $status->value['revision']->getPage() ),
array( 'log_id' => $logId ),
__METHOD__
);
$dbw->commit(); // commit before anything bad can happen
}
}
wfProfileOut( __METHOD__ . '-edit' );
# Commit the transaction now, in case something goes wrong later
# The most important thing is that files don't get lost, especially archives
$dbw->commit( __METHOD__ );
# Save to cache and purge the squid
# We shall not saveToCache before the commit since otherwise
# in case of a rollback there is an usable file from memcached