On 09/17/2014 12:19 AM, Jonathan Nieder wrote:
Michael Haggerty wrote:
If closing an open lockfile fails, then we cannot be sure of the
contents of the lockfile
Is that true? It seems more like a bug in close_lock_file: if it
fails, perhaps it should either set lk-fd back to fd or unlink the
lockfile itself.
From close(2) (note especially the last sentence):
Not checking the return value of close() is a common but nevertheless serious
programming
error. It is quite possible that errors on a previous write(2)
operation are first
reported at the final close(). Not checking the return value when closing
the file may
lead to silent loss of data. This can especially be observed with NFS
and with disk
quota. Note that the return value should only be used for diagnostics.
In particular
close() should not be retried after an EINTR since this may cause a reused
descriptor from
another thread to be closed.
From that I conclude that if close() fails, pretty much all bets are off
about the contents of the lockfile. So we wouldn't want to set lk-fd
back to fd.
But finishing the rollback itself might be an alternative...
What do other callers do on close_lock_file failure?
From what I can see, the only callers that don't die() immediately are
the following (which call close_lock_file() directly or indirectly via
write_locked_index()):
try_merge_strategy(): returns an error. It looks like this could end up
reusing the same lock_file object before it had been rolled back -
would be improved if close_lock_file() would rollback on failure.
write_cache_as_tree(): does a rollback - wouldn't mind an automatic
rollback.
merge_recursive_generic(): returns an error, and caller exits
immediately - wouldn't mind an automatic rollback.
So, I will change close_lock_file() to roll back on errors.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
