diff --git a/Makefile b/Makefile index 617475622b98db..2e3fb5b8d18e30 100644 --- a/Makefile +++ b/Makefile @@ -729,6 +729,7 @@ TEST_BUILTINS_OBJS += test-strcmp-offset.o TEST_BUILTINS_OBJS += test-string-list.o TEST_BUILTINS_OBJS += test-submodule-config.o TEST_BUILTINS_OBJS += test-subprocess.o +TEST_BUILTINS_OBJS += test-trace.o TEST_BUILTINS_OBJS += test-urlmatch-normalization.o TEST_BUILTINS_OBJS += test-wildmatch.o TEST_BUILTINS_OBJS += test-write-cache.o diff --git a/compat/mingw.c b/compat/mingw.c index 6ded1c859f1b5a..6da9ce86174c94 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -514,6 +514,25 @@ int mingw_chmod(const char *filename, int mode) return _wchmod(wfilename, mode); } +int mingw_lock_or_unlock_fd_for_appending(int fd, int lock_it) +{ + HANDLE handle = (HANDLE)_get_osfhandle(fd); + OVERLAPPED overlapped = { 0 }; + DWORD err; + + if (!lock_it && UnlockFileEx(handle, 0, -1, 0, &overlapped)) + return 0; + if (lock_it && + LockFileEx(handle, LOCKFILE_EXCLUSIVE_LOCK, 0, -1, 0, &overlapped)) + return 0; + + err = GetLastError(); + /* LockFileEx() cannot lock pipes */ + errno = err == ERROR_INVALID_FUNCTION ? + EBADF : err_win_to_posix(GetLastError()); + return -1; +} + /* * The unit of FILETIME is 100-nanoseconds since January 1, 1601, UTC. * Returns the 100-nanoseconds ("hekto nanoseconds") since the epoch. diff --git a/compat/mingw.h b/compat/mingw.h index 571019d0bddcea..0f76d89a9df3f6 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -397,6 +397,9 @@ HANDLE winansi_get_osfhandle(int fd); * git specific compatibility */ +int mingw_lock_or_unlock_fd_for_appending(int fd, int lock_it); +#define lock_or_unlock_fd_for_appending mingw_lock_or_unlock_fd_for_appending + #define has_dos_drive_prefix(path) \ (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0) int mingw_skip_dos_drive_prefix(char **path); diff --git a/git-compat-util.h b/git-compat-util.h index 9a64998b24b1de..13b83badea6a51 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1202,6 +1202,8 @@ struct tm *git_gmtime_r(const time_t *, struct tm *); #define getc_unlocked(fh) getc(fh) #endif +extern int lock_or_unlock_fd_for_appending(int fd, int lock_it); + /* * Our code often opens a path to an optional file, to work on its * contents when we can successfully open it. We can ignore a failure diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c index 805a45de9c877d..7adce872bcdf21 100644 --- a/t/helper/test-tool.c +++ b/t/helper/test-tool.c @@ -39,6 +39,7 @@ static struct test_cmd cmds[] = { { "string-list", cmd__string_list }, { "submodule-config", cmd__submodule_config }, { "subprocess", cmd__subprocess }, + { "trace", cmd__trace }, { "urlmatch-normalization", cmd__urlmatch_normalization }, { "wildmatch", cmd__wildmatch }, { "write-cache", cmd__write_cache }, diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index 7116ddfb94398d..c462ac924e723d 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -33,6 +33,7 @@ int cmd__strcmp_offset(int argc, const char **argv); int cmd__string_list(int argc, const char **argv); int cmd__submodule_config(int argc, const char **argv); int cmd__subprocess(int argc, const char **argv); +int cmd__trace(int argc, const char **argv); int cmd__urlmatch_normalization(int argc, const char **argv); int cmd__wildmatch(int argc, const char **argv); int cmd__write_cache(int argc, const char **argv); diff --git a/t/helper/test-trace.c b/t/helper/test-trace.c new file mode 100644 index 00000000000000..04159c77a7feb7 --- /dev/null +++ b/t/helper/test-trace.c @@ -0,0 +1,130 @@ +#include "test-tool.h" +#include "cache.h" +#include "run-command.h" + +static struct child_process children[2] = { + CHILD_PROCESS_INIT, + CHILD_PROCESS_INIT, +}; + +#define SAY(child, what) \ + if (write_in_full(children[child].in, \ + what "\n", strlen(what) + 1) < 0) \ + die("Failed to tell child process #%d to %s", child, what) + +#define LISTEN(child, what) \ + if (strbuf_getwholeline_fd(&buf, children[child].out, '\n') < 0) \ + die("Child process #%d failed to acknowledge %s", child, what) + +#define ACK(what) \ + if (write_in_full(1, what ": ACK\n", strlen(what) + 6) < 0) \ + die_errno("'%s': %s ACK", child_name, what) + +static void contention_orchestrator(const char *argv0) +{ + struct strbuf buf = STRBUF_INIT; + int i; + + /* Spawn two children and simulate write contention */ + trace_printf("start"); + + for (i = 0; i < 2; i++) { + strbuf_reset(&buf); + strbuf_addf(&buf, "child #%d", i); + argv_array_pushl(&children[i].args, + argv0, "trace", "lock", buf.buf, NULL); + children[i].in = children[i].out = -1; + if (start_command(&children[i]) < 0) + die("Could not spawn child process #%d", i); + } + + SAY(1, "lock"); + LISTEN(1, "lock"); + + SAY(0, "trace delayed"); + SAY(1, "trace while-locked"); + LISTEN(1, "trace"); + + SAY(1, "unlock"); + LISTEN(1, "unlock"); + LISTEN(0, "trace"); + + SAY(0, "quit"); + SAY(1, "quit"); + + if (finish_command(&children[0]) < 0 || + finish_command(&children[1]) < 0) + die("Child process failed to finish"); + + strbuf_release(&buf); +} + +static void child(const char *child_name) +{ + struct strbuf buf = STRBUF_INIT; + int fd, locked = 0; + const char *p; + + /* This is the child process */ + trace_printf("child start: '%s'", child_name); + fd = trace_default_key.fd; + if (fd <= 0) + die("child process: not tracing..."); + while (!strbuf_getwholeline_fd(&buf, 0, '\n')) { + strbuf_rtrim(&buf); + if (!strcmp("lock", buf.buf)) { + if (lock_or_unlock_fd_for_appending(fd, 1) < 0 && + errno != EBADF && errno != EINVAL) + die_errno("'%s': lock", child_name); + ACK("lock"); + locked = 1; + } else if (!strcmp("unlock", buf.buf)) { + if (lock_or_unlock_fd_for_appending(fd, 0) < 0 && + errno != EBADF && errno != EINVAL) + die_errno("'%s': unlock", child_name); + ACK("unlock"); + locked = 0; + } else if (skip_prefix(buf.buf, "trace ", &p)) { + if (!locked) + trace_printf("%s: %s", child_name, p); + else { + char *p2 = xstrdup(p); + + /* Give the racy process a run for its money. */ + sleep_millisec(50); + + strbuf_reset(&buf); + strbuf_addf(&buf, "%s: %s\n", + child_name, p2); + free(p2); + + if (write_in_full(fd, buf.buf, buf.len) < 0) + die_errno("'%s': trace", child_name); + } + ACK("trace"); + } else if (!strcmp("quit", buf.buf)) { + close(0); + close(1); + break; + } else + die("Unhandled command: '%s'", buf.buf); + + } + + strbuf_release(&buf); +} + +int cmd__trace(int argc, const char **argv) +{ + const char *argv0 = argv[-1]; + + if (argc > 1 && !strcmp("lock", argv[1])) { + if (argc > 2) + child(argv[2]); + else + contention_orchestrator(argv0); + } else + die("Usage: %s %s lock []", argv0, argv[0]); + + return 0; +} diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh index 23fbe6434abd3f..57f7a1246046a4 100755 --- a/t/t0070-fundamental.sh +++ b/t/t0070-fundamental.sh @@ -34,4 +34,10 @@ test_expect_success 'check for a bug in the regex routines' ' test-tool regex --bug ' +test_expect_success 'multiple processes can GIT_TRACE to the same file' ' + GIT_TRACE="$(pwd)/trace" test-tool trace lock && + sed -n "/while-locked/,\$s/.*delayed$/YES/p" actual && + test YES = "$(cat actual)" +' + test_done diff --git a/trace.c b/trace.c index fc623e91fdd7ed..16abdb816a7d53 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) diff --git a/wrapper.c b/wrapper.c index e4fa9d84cd0767..5aecbda3402f3c 100644 --- a/wrapper.c +++ b/wrapper.c @@ -690,3 +690,19 @@ int xgethostname(char *buf, size_t len) buf[len - 1] = 0; return ret; } + +#ifndef GIT_WINDOWS_NATIVE +int lock_or_unlock_fd_for_appending(int fd, int lock_it) +{ + struct flock flock; + + flock.l_type = lock_it ? F_WRLCK : F_UNLCK; + + /* (un-)lock the whole file */ + flock.l_whence = SEEK_SET; + flock.l_start = 0; + flock.l_len = 0; + + return fcntl(fd, F_SETLKW, &flock); +} +#endif