# HG changeset patch # User Vadim Gelfer # Date 1143220092 28800 # Node ID 65cc17ae964999792feb0c2b3da30494e303c3d8 # Parent 802e8a029d99f1e6a61834146ffe503c87aaad30 fix race in localrepo.addchangegroup. localrepo.addchangegroup writes to changelog, then manifest, then normal files. this breaks access ordering. if reader reads changelog while manifest is being written, can find pointers into places in manifest that are not yet written. same can happen for manifest and normal files. fix is to make almost no change to localrepo.addchangegroup. it must to write changelog and manifest data early because it has to read them while writing other files. instead, write changelog and manifest data to temp file that reader cannot see, then append temp data to manifest after all normal files written, finally append temp data to changelog. temp file code is in new appendfile module. can be used in other places with small changes. much smaller race still left. we write all new data in one write call, but reader can maybe see partial update because python or os or filesystem cannot always make write really atomic. file locking no help: slow, not portable, not reliable over nfs. only real safe other plan is write to temp file every time and rename, but performance bad when manifest or changelog is big. diff -r 802e8a029d99 -r 65cc17ae9649 mercurial/localrepo.py --- a/mercurial/localrepo.py Fri Mar 24 15:19:08 2006 +0100 +++ b/mercurial/localrepo.py Fri Mar 24 09:08:12 2006 -0800 @@ -11,7 +11,7 @@ from i18n import gettext as _ from demandload import * demandload(globals(), "re lock transaction tempfile stat mdiff errno ui") -demandload(globals(), "changegroup") +demandload(globals(), "appendfile changegroup") class localrepository(object): def __del__(self): @@ -1343,10 +1343,10 @@ def csmap(x): self.ui.debug(_("add changeset %s\n") % short(x)) - return self.changelog.count() + return cl.count() def revmap(x): - return self.changelog.rev(x) + return cl.rev(x) if not source: return @@ -1357,23 +1357,29 @@ tr = self.transaction() - oldheads = len(self.changelog.heads()) + # write changelog and manifest data to temp files so + # concurrent readers will not see inconsistent view + cl = appendfile.appendchangelog(self.opener) + + oldheads = len(cl.heads()) # pull off the changeset group self.ui.status(_("adding changesets\n")) - co = self.changelog.tip() + co = cl.tip() chunkiter = changegroup.chunkiter(source) - cn = self.changelog.addgroup(chunkiter, csmap, tr, 1) # unique - cnr, cor = map(self.changelog.rev, (cn, co)) + cn = cl.addgroup(chunkiter, csmap, tr, 1) # unique + cnr, cor = map(cl.rev, (cn, co)) if cn == nullid: cnr = cor changesets = cnr - cor + mf = appendfile.appendmanifest(self.opener) + # pull off the manifest group self.ui.status(_("adding manifests\n")) - mm = self.manifest.tip() + mm = mf.tip() chunkiter = changegroup.chunkiter(source) - mo = self.manifest.addgroup(chunkiter, revmap, tr) + mo = mf.addgroup(chunkiter, revmap, tr) # process the files self.ui.status(_("adding file changes\n")) @@ -1389,6 +1395,15 @@ revisions += fl.count() - o files += 1 + # write order here is important so concurrent readers will see + # consistent view of repo + mf.writedata() + cl.writedata() + + # make changelog and manifest see real files again + self.changelog = changelog.changelog(self.opener) + self.manifest = manifest.manifest(self.opener) + newheads = len(self.changelog.heads()) heads = "" if oldheads and newheads > oldheads: