Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
Open the lock files close-on-exec
Each exec filter and snapshot compressor a fill runs inherited the
slot's lock descriptor beside the stdout it writes into. An flock lasts
until its last copy closes and nothing waits for a process a filter
leaves behind, so a fill killed before its rename could leave the slot
locked, and served stale or uncached, for as long as such a process
lived.
The scan holds its lock across no exec and opens the same way so that
the two stay alike.
| -rw-r--r-- | source/cache.c | 4 | |
| -rw-r--r-- | source/cgit.c | 2 | |
| -rwxr-xr-x | tests/t0003-cache.sh | 27 |
3 files changed, 31 insertions, 2 deletions
diff --git a/source/cache.c b/source/cache.c index 4802ab5..12151b8 100644 --- a/source/cache.c +++ b/source/cache.c @@ -202,7 +202,9 @@ static int lock_slot(struct cache_slot *slot) { struct stat held, named; - slot->lock_fd = open(slot->lock_path, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR); + // Close-on-exec, so that a program the fill runs gets the file only + // as its stdout, and holds the lock no longer than it can write there. + slot->lock_fd = open(slot->lock_path, O_RDWR | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR); if (slot->lock_fd == -1) return errno; // An flock rather than a POSIX record lock. The descriptor is put on diff --git a/source/cgit.c b/source/cgit.c index f81af79..f5f3138 100644 --- a/source/cgit.c +++ b/source/cgit.c @@ -337,7 +337,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc) FILE *f; strbuf_addf(&locked_rc, "%s.lock", cached_rc); - fd = open(locked_rc.buf, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR); + fd = open(locked_rc.buf, O_RDWR | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR); if (fd == -1) { err = errno; fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", locked_rc.buf, strerror(err), err); diff --git a/tests/t0003-cache.sh b/tests/t0003-cache.sh index 7e92b05..945ac63 100755 --- a/tests/t0003-cache.sh +++ b/tests/t0003-cache.sh @@ -267,4 +267,31 @@ test_expect_success 'a slot stays locked while an exec filter runs' ' ! grep "Unable to publish" first.err ' +# An flock lasts until the last copy of its descriptor closes, so a filter +# may hold the slot's lock file only as the stdout it writes the page to. A +# stray copy would keep the slot locked for as long as the filter, or +# anything it leaves running, lives on. +test_expect_success PERL 'a filter holds the lock file only as its stdout' ' + rm -rf cache3 && mkdir cache3 && + write_script fd-filter "$PERL_PATH" <<-\EOF && + use POSIX qw(fstat); + 1 while <STDIN>; + my @lock = stat $ENV{LOCK_FILE} or die "$!\n"; + my @fds = grep { + my @fd = fstat($_); + @fd && $fd[0] == $lock[0] && $fd[1] == $lock[1] + } 0 .. 255; + print "lock file on fd @fds.\n"; + EOF + { + echo "cache-size=1" && + sed "/^cache-size=/d" bigkeyrc && + echo "repo.readme=:file-1" && + echo "repo.about-filter=exec:$PWD/fd-filter" + } >fdrc && + LOCK_FILE="$PWD/cache3/00000000.lock" CGIT_CONFIG="$PWD/fdrc" \ + QUERY_STRING="url=bigpage/about/" cgit >fd.out && + grep -F "lock file on fd 1." fd.out +' + test_done |