[darcs-devel] darcs patch: Remove redundant sift_for_pending (and 1 more)

[David Roundy]

On Wed, Jun 22, 2005 at 06:48:44PM +0100, Ian Lynagh wrote:
> On Wed, Jun 22, 2005 at 08:28:07AM -0400, David Roundy wrote:
> > On Wed, Jun 22, 2005 at 04:34:42AM +0100, Ian Lynagh wrote:
> > > 
> > > Do you have any comments about this for a solution to #323?
> > 
> > Looks mostly all right.  My biggest concern (and I haven't actually applied
> > it, so I don't see all the context) is that it looks like the update of the
> > inventories is now further from the application to pristine.  I'd rather
> > put them adjacent to minimize the danger of repository corruption due to
> > power outage or kill -9.
> 
> Good point. I think doing the inventory update last would make sense,
> and make some current odd behaviour when a patch fails to apply be less
> odd (when you pull again there will be something to pull, and doing so
> will fail, rather than you just being told there are no changes).

Makes sense to me.

> > I do think that we still need to track down that bus error, right?
> 
> No. The bus error is caused by something like this (this is partly by
> assumption/guesswork rather than tracing down all the little details):
> 
> Call get_unrecorded (yielding mp) which opens and mmaps the file in
> pristine and makes a lazy suspension to do the diff with the file in
> working (or, in this case, no file).
> 
> Apply the patch to pristine, removing the file
> 
> Try to evaluate the diff, which requires reading the mmapped data which
> now points at a non-existant file. SIGBUS.

The catch with this explanation is that I don't believe it works that way.
If you remove an mmapped file, that mmap remains valid--the file isn't
actually deleted until it's closed.  The only way to get the SIGBUS is if
we truncate the file prior to moving it, but we shouldn't do that--the
policy (possibly not always implemented) is to always modify files by
creating a new file and renaming, which leaves mmaps intact (as file
removal does).
-- 
David Roundy
http://www.darcs.net