From 4257184a0fd276af7e1741dda8a7468a30b4a9c6 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 11 Dec 2019 10:40:01 -0500 Subject: [PATCH] 15928: Fix deadlock. Locking the parent dir isn't necessary to finish async writes, and deadlocks if a waitPrune() is waiting for the async writes to finish. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- sdk/go/arvados/fs_collection.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/sdk/go/arvados/fs_collection.go b/sdk/go/arvados/fs_collection.go index 3d0928b84e..d0e97f2ad1 100644 --- a/sdk/go/arvados/fs_collection.go +++ b/sdk/go/arvados/fs_collection.go @@ -698,21 +698,9 @@ func (dn *dirnode) commitBlock(ctx context.Context, refs []fnSegmentRef, bufsize go func() { defer close(done) defer close(errs) - locked := map[*filenode]bool{} locator, _, err := dn.fs.PutB(block) dn.fs.throttle().Release() { - if !sync { - dn.Lock() - defer dn.Unlock() - for _, name := range dn.sortedNames() { - if fn, ok := dn.inodes[name].(*filenode); ok { - fn.Lock() - defer fn.Unlock() - locked[fn] = true - } - } - } defer func() { for _, seg := range segs { if seg.flushing == done { @@ -727,6 +715,7 @@ func (dn *dirnode) commitBlock(ctx context.Context, refs []fnSegmentRef, bufsize } for idx, ref := range refs { if !sync { + ref.fn.Lock() // In async mode, fn's lock was // released while we were waiting for // PutB(); lots of things might have @@ -735,17 +724,15 @@ func (dn *dirnode) commitBlock(ctx context.Context, refs []fnSegmentRef, bufsize // file segments have // rearranged or changed in // some way + ref.fn.Unlock() continue } else if seg, ok := ref.fn.segments[ref.idx].(*memSegment); !ok || seg != segs[idx] { // segment has been replaced + ref.fn.Unlock() continue } else if seg.flushing != done { // seg.buf has been replaced - continue - } else if !locked[ref.fn] { - // file was renamed, moved, or - // deleted since we called - // PutB + ref.fn.Unlock() continue } } @@ -763,6 +750,9 @@ func (dn *dirnode) commitBlock(ctx context.Context, refs []fnSegmentRef, bufsize // lock, writing different segments from the // same file. atomic.AddInt64(&ref.fn.memsize, -int64(len(data))) + if !sync { + ref.fn.Unlock() + } } }() if sync { -- 2.30.2