@matheusbsbs / matheus-bagliano / commits / 1fb0c80985

log_ref_setup(): improve robustness against races

Change log_ref_setup() to use raceproof_create_file() to create the new logfile. This makes it more robust against a race against another process that might be trying to clean up empty directories while we are trying to create a new logfile. This also means that it will only call create_leading_directories() if open() fails, which should be a net win. Even in the cases where we are willing to create a new logfile, it will usually be the case that the logfile already exists, or if not then that the directory containing the logfile already exists. In such cases, we will save some work that was previously done unconditionally. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Jan 6, 2017 at 17:22 UTC 1fb0c809859252f037286d32f56432444d65bd38
1 file changed +18 -23
refs/files-backend.c
+18 -23
@@ -2710,6 +2710,14 @@ static int commit_ref(struct ref_lock *lock)
2710 return 0;
2711 }
2712
2713 +static int open_or_create_logfile(const char *path, void *cb)
2714 +{
2715 + int *fd = cb;
2716 +
2717 + *fd = open(path, O_APPEND | O_WRONLY | O_CREAT, 0666);
2718 + return (*fd < 0) ? -1 : 0;
2719 +}
2720 +
2721 /*
2722 * Create a reflog for a ref. If force_create = 0, the reflog will
2723 * only be created for certain refs (those for which
@@ -2723,31 +2731,18 @@ static int log_ref_setup(const char *refname, struct strbuf *logfile, struct str
2731 strbuf_git_path(logfile, "logs/%s", refname);
2732
2733 if (force_create || should_autocreate_reflog(refname)) {
2726 - if (safe_create_leading_directories(logfile->buf) < 0) {
2727 - strbuf_addf(err, "unable to create directory for '%s': "
2728 - "%s", logfile->buf, strerror(errno));
2729 - return -1;
2730 - }
2731 - logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
2732 - if (logfd < 0) {
2733 - if (errno == EISDIR) {
2734 - /*
2735 - * The directory that is in the way might be
2736 - * empty. Try to remove it.
2737 - */
2738 - if (remove_empty_directories(logfile)) {
2739 - strbuf_addf(err, "there are still logs under "
2740 - "'%s'", logfile->buf);
2741 - return -1;
2742 - }
2743 - logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
2744 - }
2745 -
2746 - if (logfd < 0) {
2734 + if (raceproof_create_file(logfile->buf, open_or_create_logfile, &logfd)) {
2735 + if (errno == ENOENT)
2736 + strbuf_addf(err, "unable to create directory for '%s': "
2737 + "%s", logfile->buf, strerror(errno));
2738 + else if (errno == EISDIR)
2739 + strbuf_addf(err, "there are still logs under '%s'",
2740 + logfile->buf);
2741 + else
2742 strbuf_addf(err, "unable to append to '%s': %s",
2743 logfile->buf, strerror(errno));
2749 - return -1;
2750 - }
2744 +
2745 + return -1;
2746 }
2747 } else {
2748 logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);