[issue1321] External code should not see uncommitted transactions about to roll back

[Jesse Glick]

New submission from Jesse Glick <jesse.glick at sun.com>:

It seems to be a flaw in Hg's design that while pretxn* hooks are running,
external clients can pull changesets which are about to be rejected:

http://hgbook.red-bean.com/hgbookch10.html#x14-20400010.3

This could "contaminate" clients with changesets they should not have. The
suggested "gatekeeper" workaround is extremely clumsy and sounds quite inefficient.

Restrict our attention to pretxnchangegroup, which seems the more important to
me (since commits normally happen on a developer's machine, who can control who
is pulling from the clone). localrepo.addchangegroup calls cl.finalize, then
self.hook('pretxnchangegroup',...), then tr.close. There are three cases:

1. There are no pretxnchangegroup hooks registered. Then there is no problem
with the current code - we can expect the transaction to commit, barring a
filesystem failure, process death, etc. (and even in these cases there is no
real problem because any changesets which do get written are in fact OK).

2. There are some hooks registered, but they are all in-process (python:...). In
this case I think it would not matter if cl.finalize were moved after the hook
calls - the hooks would still "see" the new changesets, assuming they are just
using the repo object passed to them.

3. There are some external hooks registered (i.e. shell commands). In this case
it would be necessary for 00.changelog.i to still be the old known-good copy,
and have some separate file containing the speculative changegroup.
changelog.finalize already has code to use a rename from 00.changelog.i.a rather
than append during a clone, but it might be more efficient to just write
_delaybuf to some file like 00.changelog.i.extra. Run the external hooks with an
env var set (e.g. HG_READ_UNCOMMITTED=true) which would be interpreted by
changelog to mean to read 00.changelog.i.extra after 00.changelog.i. If all
hooks succeed, just append _delaybuf to 00.changelog.i as usual. When the
transaction terminates (in either commit or abort), delete 00.changelog.i.extra.

With this scheme, hooks would see the uncommitted changesets, but no one else would.

----------
messages: 7285
nosy: jglick
priority: wish
status: unread
title: External code should not see uncommitted transactions about to roll back

____________________________________________________
Mercurial issue tracker <mercurial-bugs at selenic.com>
<http://www.selenic.com/mercurial/bts/issue1321>
____________________________________________________