Skip to content
  • Jeff King's avatar
    bundle: dup() output descriptor closer to point-of-use · 2c8ee1f5
    Jeff King authored and Junio C Hamano's avatar Junio C Hamano committed
    
    
    When writing a bundle to a file, the bundle code actually creates
    "your.bundle.lock" using our lockfile interface. We feed that output
    descriptor to a child git-pack-objects via run-command, which has the
    quirk that it closes the output descriptor in the parent.
    
    To avoid confusing the lockfile code (which still thinks the descriptor
    is valid), we dup() it, and operate on the duplicate.
    
    However, this has a confusing side effect: after the dup() but before we
    call pack-objects, we have _two_ descriptors open to the lockfile. If we
    call die() during that time, the lockfile code will try to clean up the
    partially-written file. It knows to close() the file before unlinking,
    since on some platforms (i.e., Windows) the open file would block the
    deletion. But it doesn't know about the duplicate descriptor. On
    Windows, triggering an error at the right part of the code will result
    in the cleanup failing and the lockfile being left in the filesystem.
    
    We can solve this by moving the dup() much closer to start_command(),
    shrinking the window in which we have the second descriptor open. It's
    easy to place this in such a way that no die() is possible. We could
    still die due to a signal in the exact wrong moment, but we already
    tolerate races there (e.g., a signal could come before we manage to put
    the file on the cleanup list in the first place).
    
    As a bonus, this shields create_bundle() itself from the duplicate-fd
    trick, and we can simplify its error handling (note that the lock
    rollback now happens unconditionally, but that's OK; it's a noop if we
    didn't open the lock in the first place).
    
    The included test uses an empty bundle to cause a failure at the right
    spot in the code, because that's easy to trigger (the other likely
    errors are write() problems like ENOSPC).  Note that it would already
    pass on non-Windows systems (because they are happy to unlink an
    already-open file).
    
    Based-on-a-patch-by: default avatarGaël Lhez <gael.lhez@gmail.com>
    Signed-off-by: default avatarJeff King <peff@peff.net>
    Tested-by: default avatarJohannes Schindelin <johannes.schindelin@gmx.de>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
    2c8ee1f5