[PATCH v2 3/4] trace: lock the trace file to avoid racy trace_write() calls

["Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When multiple processes try to write to the same file, it is not
guaranteed that everything works as expected: those writes can overlap,
and in the worst case even lose messages.

This happens in t/t5552-skipping-fetch-negotiator.sh, where we abuse the
`GIT_TRACE` facility to write traces of two concurrent processes (`git
fetch` and `git upload-pack`) to the same file, and then verify that the
trace contains certain expected breadcrumbs.

To remedy this, let's lock the file descriptors for exclusive writing,
using the function we just introduced in the previous commit.

Note: while the POSIX documentation of fcntl() at
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
suggests that the `errno` is set to `EINVAL` when being asked to
lock a file descriptor that cannot be locked, on macOS it results in
`EBADF` when trying to lock a redirected `stdout` (which the
documentation claims should indicate that the file descriptor is not
valid for writing).

To cover all our bases, we simply treat both `EINVAL` and `EBADF` as
indicators that we cannot lock/unlock this file descriptor.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 trace.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/trace.c b/trace.c
index fc623e91f..16abdb816 100644
--- a/trace.c
+++ b/trace.c
@@ -114,11 +114,20 @@ static int prepare_trace_line(const char *file, int line,
 
 static void trace_write(struct trace_key *key, const void *buf, unsigned len)
 {
-	if (write_in_full(get_trace_fd(key), buf, len) < 0) {
+	int fd = get_trace_fd(key), locked;
+
+	locked = !lock_or_unlock_fd_for_appending(fd, 1);
+	if (!locked && errno != EBADF && errno != EINVAL)
+		warning("unable to lock file descriptor for %s: %s",
+			key->key, strerror(errno));
+	if (write_in_full(fd, buf, len) < 0) {
 		warning("unable to write trace for %s: %s",
 			key->key, strerror(errno));
 		trace_disable(key);
 	}
+	if (locked && lock_or_unlock_fd_for_appending(fd, 0) < 0)
+		warning("failed to remove lock on fd for %s: %s",
+			key->key, strerror(errno));
 }
 
 void trace_verbatim(struct trace_key *key, const void *buf, unsigned len)
-- 
gitgitgadget