From 61a1140dd2358eec5eab1c85038c5a4e079f11b0 Mon Sep 17 00:00:00 2001 From: Ibrahim Jarif Date: Mon, 23 Sep 2019 23:20:27 +0530 Subject: [PATCH 1/2] Acquire lock before unmapping vlog files When we truncate the vlog file, we unmap and remap it. This leaves a window for segfault. This PR reintroduces the lock that got removed in commit https://github.com/dgraph-io/badger/commit/afa8faea. --- value.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/value.go b/value.go index a3a091b86..cbd1a3aaf 100644 --- a/value.go +++ b/value.go @@ -141,6 +141,13 @@ func (lf *logFile) doneWriting(offset uint32) error { return errors.Wrapf(err, "Unable to sync value log: %q", lf.path) } + // Before we were acquiring a lock here on lf.lock, because we were invalidating the file + // descriptor due to reopening it as read-only. Now, we don't invalidate the fd, but unmap it, + // truncate it and remap it. That creates a window where we have segfaults because the mmap is + // no longer valid, while someone might be reading it. Therefore, we need a lock here again. + lf.lock.Lock() + defer lf.lock.Unlock() + // Unmap file before we truncate it. Windows cannot truncate a file that is mmapped. if err := lf.munmap(); err != nil { return errors.Wrapf(err, "failed to mumap vlog file %s", lf.fd.Name()) From 1e2b78e09f28b10520738533fccf898de62be538 Mon Sep 17 00:00:00 2001 From: Ibrahim Jarif Date: Mon, 23 Sep 2019 23:22:32 +0530 Subject: [PATCH 2/2] Fix typo --- value.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/value.go b/value.go index cbd1a3aaf..c915098ad 100644 --- a/value.go +++ b/value.go @@ -150,7 +150,7 @@ func (lf *logFile) doneWriting(offset uint32) error { // Unmap file before we truncate it. Windows cannot truncate a file that is mmapped. if err := lf.munmap(); err != nil { - return errors.Wrapf(err, "failed to mumap vlog file %s", lf.fd.Name()) + return errors.Wrapf(err, "failed to munmap vlog file %s", lf.fd.Name()) } // TODO: Confirm if we need to run a file sync after truncation.