git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	David Turner <novalis@novalis.org>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH v3 05/23] raceproof_create_file(): new function
Date: Sat, 31 Dec 2016 04:12:45 +0100	[thread overview]
Message-ID: <f933f9d3c4c53b42ecc75b7a743ed4bfd390b4c5.1483153436.git.mhagger@alum.mit.edu> (raw)
In-Reply-To: <cover.1483153436.git.mhagger@alum.mit.edu>

Add a function that tries to create a file and any containing
directories in a way that is robust against races with other processes
that might be cleaning up empty directories at the same time.

The actual file creation is done by a callback function, which, if it
fails, should set errno to EISDIR or ENOENT according to the convention
of open(). raceproof_create_file() detects such failures, and
respectively either tries to delete empty directories that might be in
the way of the file or tries to create the containing directories. Then
it retries the callback function.

This function is not yet used.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache.h     | 43 ++++++++++++++++++++++++++++++++++++++
 sha1_file.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

diff --git a/cache.h b/cache.h
index 8177c3a..7bc4ea6 100644
--- a/cache.h
+++ b/cache.h
@@ -1057,6 +1057,49 @@ enum scld_error {
 enum scld_error safe_create_leading_directories(char *path);
 enum scld_error safe_create_leading_directories_const(const char *path);
 
+/*
+ * Callback function for raceproof_create_file(). This function is
+ * expected to do something that makes dirname(path) permanent despite
+ * the fact that other processes might be cleaning up empty
+ * directories at the same time. Usually it will create a file named
+ * path, but alternatively it could create another file in that
+ * directory, or even chdir() into that directory. The function should
+ * return 0 if the action was completed successfully. On error, it
+ * should return a nonzero result and set errno.
+ * raceproof_create_file() treats two errno values specially:
+ *
+ * - ENOENT -- dirname(path) does not exist. In this case,
+ *             raceproof_create_file() tries creating dirname(path)
+ *             (and any parent directories, if necessary) and calls
+ *             the function again.
+ *
+ * - EISDIR -- the file already exists and is a directory. In this
+ *             case, raceproof_create_file() deletes the directory
+ *             (recursively) if it is empty and calls the function
+ *             again.
+ *
+ * Any other errno causes raceproof_create_file() to fail with the
+ * callback's return value and errno.
+ *
+ * Obviously, this function should be OK with being called again if it
+ * fails with ENOENT or EISDIR. In other scenarios it will not be
+ * called again.
+ */
+typedef int create_file_fn(const char *path, void *cb);
+
+/*
+ * Create a file in dirname(path) by calling fn, creating leading
+ * directories if necessary. Retry a few times in case we are racing
+ * with another process that is trying to clean up the directory that
+ * contains path. See the documentation for create_file_fn for more
+ * details.
+ *
+ * Return the value and set the errno that resulted from the most
+ * recent call of fn. fn is always called at least once, and will be
+ * called more than once if it returns ENOENT or EISDIR.
+ */
+int raceproof_create_file(const char *path, create_file_fn fn, void *cb);
+
 int mkdir_in_gitdir(const char *path);
 extern char *expand_user_path(const char *path);
 const char *enter_repo(const char *path, int strict);
diff --git a/sha1_file.c b/sha1_file.c
index ae8f0b4..b08f54c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -179,6 +179,74 @@ enum scld_error safe_create_leading_directories_const(const char *path)
 	return result;
 }
 
+int raceproof_create_file(const char *path, create_file_fn fn, void *cb)
+{
+	/*
+	 * The number of times we will try to remove empty directories
+	 * in the way of path. This is only 1 because if another
+	 * process is racily creating directories that conflict with
+	 * us, we don't want to fight against them.
+	 */
+	int remove_directories_remaining = 1;
+
+	/*
+	 * The number of times that we will try to create the
+	 * directories containing path. We are willing to attempt this
+	 * more than once, because another process could be trying to
+	 * clean up empty directories at the same time as we are
+	 * trying to create them.
+	 */
+	int create_directories_remaining = 3;
+
+	/* A scratch copy of path, filled lazily if we need it: */
+	struct strbuf path_copy = STRBUF_INIT;
+
+	int ret, save_errno;
+
+	/* Sanity check: */
+	assert(*path);
+
+retry_fn:
+	ret = fn(path, cb);
+	save_errno = errno;
+	if (!ret)
+		goto out;
+
+	if (errno == EISDIR && remove_directories_remaining-- > 0) {
+		/*
+		 * A directory is in the way. Maybe it is empty; try
+		 * to remove it:
+		 */
+		if (!path_copy.len)
+			strbuf_addstr(&path_copy, path);
+
+		if (!remove_dir_recursively(&path_copy, REMOVE_DIR_EMPTY_ONLY))
+			goto retry_fn;
+	} else if (errno == ENOENT && create_directories_remaining-- > 0) {
+		/*
+		 * Maybe the containing directory didn't exist, or
+		 * maybe it was just deleted by a process that is
+		 * racing with us to clean up empty directories. Try
+		 * to create it:
+		 */
+		enum scld_error scld_result;
+
+		if (!path_copy.len)
+			strbuf_addstr(&path_copy, path);
+
+		do {
+			scld_result = safe_create_leading_directories(path_copy.buf);
+			if (scld_result == SCLD_OK)
+				goto retry_fn;
+		} while (scld_result == SCLD_VANISHED && create_directories_remaining-- > 0);
+	}
+
+out:
+	strbuf_release(&path_copy);
+	errno = save_errno;
+	return ret;
+}
+
 static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)
 {
 	int i;
-- 
2.9.3


  parent reply	other threads:[~2016-12-31  3:13 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-31  3:12 [PATCH v3 00/23] Delete directories left empty after ref deletion Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 01/23] files_rename_ref(): tidy up whitespace Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 02/23] t5505: use "for-each-ref" to test for the non-existence of references Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 03/23] safe_create_leading_directories_const(): preserve errno Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 04/23] safe_create_leading_directories(): set errno on SCLD_EXISTS Michael Haggerty
2016-12-31  3:12 ` Michael Haggerty [this message]
2016-12-31  6:11   ` [PATCH v3 05/23] raceproof_create_file(): new function Jeff King
2016-12-31  7:42     ` Michael Haggerty
2017-01-01  2:07       ` Junio C Hamano
2016-12-31  3:12 ` [PATCH v3 06/23] lock_ref_sha1_basic(): inline constant Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 07/23] lock_ref_sha1_basic(): use raceproof_create_file() Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 08/23] rename_tmp_log(): " Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 09/23] rename_tmp_log(): improve error reporting Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 10/23] log_ref_write(): inline function Michael Haggerty
2017-01-01  2:09   ` Junio C Hamano
2017-01-01  8:41     ` Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 11/23] log_ref_setup(): separate code for create vs non-create Michael Haggerty
2016-12-31  6:26   ` Jeff King
2016-12-31  7:52     ` Michael Haggerty
2017-01-01  3:28   ` Junio C Hamano
2017-01-01  8:45     ` Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 12/23] log_ref_setup(): improve robustness against races Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 13/23] log_ref_setup(): pass the open file descriptor back to the caller Michael Haggerty
2016-12-31  6:32   ` Jeff King
2016-12-31  7:58     ` Michael Haggerty
2016-12-31 17:58       ` Jeff King
2017-01-01 10:36         ` Junio C Hamano
2016-12-31  3:12 ` [PATCH v3 14/23] log_ref_write_1(): don't depend on logfile argument Michael Haggerty
2016-12-31  6:35   ` Jeff King
2016-12-31  8:01     ` Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 15/23] log_ref_setup(): manage the name of the reflog file internally Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 16/23] log_ref_write_1(): inline function Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 17/23] delete_ref_loose(): derive loose reference path from lock Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 18/23] delete_ref_loose(): inline function Michael Haggerty
2016-12-31  3:12 ` [PATCH v3 19/23] try_remove_empty_parents(): rename parameter "name" -> "refname" Michael Haggerty
2016-12-31  3:13 ` [PATCH v3 20/23] try_remove_empty_parents(): don't trash argument contents Michael Haggerty
2016-12-31  6:40   ` Jeff King
2017-01-02 16:27     ` Michael Haggerty
2017-01-02 17:10       ` Jeff King
2016-12-31  3:13 ` [PATCH v3 21/23] try_remove_empty_parents(): don't accommodate consecutive slashes Michael Haggerty
2017-01-01  2:30   ` Junio C Hamano
2017-01-01  5:59     ` Jeff King
2017-01-02 18:06       ` Michael Haggerty
2017-01-02 18:26         ` Jeff King
2016-12-31  3:13 ` [PATCH v3 22/23] try_remove_empty_parents(): teach to remove parents of reflogs, too Michael Haggerty
2016-12-31  3:13 ` [PATCH v3 23/23] files_transaction_commit(): clean up empty directories Michael Haggerty
2016-12-31  6:47 ` [PATCH v3 00/23] Delete directories left empty after ref deletion Jeff King
2017-01-01  2:32   ` Junio C Hamano
2017-01-01  9:24     ` Jacob Keller
2017-01-01  9:26       ` Jacob Keller
2017-01-01 12:43       ` Philip Oakley
2017-01-01 20:36         ` Jacob Keller
2017-01-02  4:19           ` Jeff King
2017-01-02 18:14             ` Michael Haggerty
2017-01-02 18:54               ` Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f933f9d3c4c53b42ecc75b7a743ed4bfd390b4c5.1483153436.git.mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=novalis@novalis.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).