From 8580b811cfbe71db190b6bc0c824ba296fbeebc1 Mon Sep 17 00:00:00 2001 From: mpl Date: Thu, 21 Apr 2016 11:14:07 -0700 Subject: [PATCH] pkg/index: make corpus RLock take a context Also remove extra (deadlocking) RLock in pkg/search. Updates issue #709 Change-Id: I556a1fbf9217f482b6a51e74c28a019dea8369a2 --- pkg/index/corpus.go | 8 ++++---- pkg/index/corpus_test.go | 22 ++++++++++++++-------- pkg/index/index.go | 2 +- pkg/search/query.go | 4 +--- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/pkg/index/corpus.go b/pkg/index/corpus.go index 6997bb835..a4267feee 100644 --- a/pkg/index/corpus.go +++ b/pkg/index/corpus.go @@ -44,7 +44,7 @@ import ( // Corpus is an in-memory summary of all of a user's blobs' metadata. type Corpus struct { mu sync.RWMutex - //mu syncutil.RWMutexTracker // when debugging + // mu syncdebug.RWMutexTracker // when debugging // building is true at start while scanning all rows in the // index. While building, certain invariants (like things @@ -108,14 +108,14 @@ type latLong struct { } // RLock locks the Corpus for reads. It must be used for any "Locked" methods. -func (c *Corpus) RLock() { c.mu.RLock() } +func (c *Corpus) RLock(ctx context.Context) { c.mu.RLock() } // RUnlock unlocks the Corpus for reads. func (c *Corpus) RUnlock() { c.mu.RUnlock() } // IsDeleted reports whether the provided blobref (of a permanode or claim) should be considered deleted. -func (c *Corpus) IsDeleted(br blob.Ref) bool { - c.RLock() +func (c *Corpus) IsDeleted(ctx context.Context, br blob.Ref) bool { + c.RLock(ctx) defer c.RUnlock() return c.IsDeletedLocked(br) } diff --git a/pkg/index/corpus_test.go b/pkg/index/corpus_test.go index 12ecd7b00..417c428ce 100644 --- a/pkg/index/corpus_test.go +++ b/pkg/index/corpus_test.go @@ -129,8 +129,9 @@ func TestCorpusAppendPermanodeAttrValues(t *testing.T) { func TestCorpusPermanodeAttrValueLocked(t *testing.T) { c, pn := newTestCorpusWithPermanode() + ctx := context.TODO() - c.RLock() + c.RLock(ctx) defer c.RUnlock() tests := []struct { @@ -174,7 +175,8 @@ func TestCorpusPermanodeAttrValueLocked(t *testing.T) { func TestCorpusPermanodeHasAttrValueLocked(t *testing.T) { c, pn := newTestCorpusWithPermanode() - c.RLock() + ctx := context.TODO() + c.RLock(ctx) defer c.RUnlock() tests := []struct { @@ -294,7 +296,8 @@ func testDeletePermanodes(t *testing.T, ch := make(chan camtypes.BlobMeta, 10) var got []camtypes.BlobMeta errc := make(chan error, 1) - c.RLock() + ctx := context.TODO() + c.RLock(ctx) go func() { errc <- enumFunc(c, context.TODO(), ch) }() for blobMeta := range ch { got = append(got, blobMeta) @@ -325,7 +328,7 @@ func testDeletePermanodes(t *testing.T, want = []blob.Ref{foopn} got = got[:0] ch = make(chan camtypes.BlobMeta, 10) - c.RLock() + c.RLock(ctx) go func() { errc <- enumFunc(c, context.TODO(), ch) }() for blobMeta := range ch { got = append(got, blobMeta) @@ -347,7 +350,7 @@ func testDeletePermanodes(t *testing.T, want = []blob.Ref{foopn, bazpn} got = got[:0] ch = make(chan camtypes.BlobMeta, 10) - c.RLock() + c.RLock(ctx) go func() { errc <- enumFunc(c, context.TODO(), ch) }() for blobMeta := range ch { got = append(got, blobMeta) @@ -432,7 +435,8 @@ func testEnumerateOrder(t *testing.T, ch := make(chan camtypes.BlobMeta, 10) var got []camtypes.BlobMeta errc := make(chan error, 1) - c.RLock() + ctx := context.TODO() + c.RLock(ctx) go func() { errc <- enumFunc(c, context.TODO(), ch) }() for blobMeta := range ch { got = append(got, blobMeta) @@ -488,11 +492,12 @@ func testCacheSortedPermanodesRace(t *testing.T, } donec <- struct{}{} }() + ctx := context.TODO() go func() { for i := 0; i < 10; i++ { ch := make(chan camtypes.BlobMeta, 10) errc := make(chan error, 1) - c.RLock() + c.RLock(ctx) go func() { errc <- enumFunc(c, context.TODO(), ch) }() for range ch { } @@ -528,7 +533,8 @@ func TestLazySortedPermanodes(t *testing.T) { enum := func(reverse bool) { ch := make(chan camtypes.BlobMeta, 10) errc := make(chan error, 1) - c.RLock() + ctx := context.TODO() + c.RLock(ctx) go func() { errc <- c.EnumeratePermanodesCreatedLocked(context.TODO(), ch, reverse) }() for range ch { } diff --git a/pkg/index/index.go b/pkg/index/index.go index 0f0ac7f54..eeaab72f3 100644 --- a/pkg/index/index.go +++ b/pkg/index/index.go @@ -1393,7 +1393,7 @@ func enumerateBlobMeta(s sorted.KeyValue, cb func(camtypes.BlobMeta) error) (err // EnumerateBlobMeta sends all metadata about all known blobs to ch and then closes ch. func (x *Index) EnumerateBlobMeta(ctx context.Context, ch chan<- camtypes.BlobMeta) (err error) { if x.corpus != nil { - x.corpus.RLock() + x.corpus.RLock(ctx) defer x.corpus.RUnlock() return x.corpus.EnumerateBlobMetaLocked(ctx, ch) } diff --git a/pkg/search/query.go b/pkg/search/query.go index 246facb37..87421d167 100644 --- a/pkg/search/query.go +++ b/pkg/search/query.go @@ -857,7 +857,7 @@ func (h *Handler) Query(rawq *SearchQuery) (*SearchResult, error) { corpus := h.corpus var unlockOnce sync.Once if corpus != nil { - corpus.RLock() + corpus.RLock(ctx) defer unlockOnce.Do(corpus.RUnlock) } @@ -943,7 +943,6 @@ func (h *Handler) Query(rawq *SearchQuery) (*SearchResult, error) { return nil, errors.New("TODO: Sorting without a corpus unsupported") } var err error - corpus.RLock() sort.Sort(sortSearchResultBlobs{res.Blobs, func(a, b *SearchResultBlob) bool { if err != nil { return false @@ -963,7 +962,6 @@ func (h *Handler) Query(rawq *SearchQuery) (*SearchResult, error) { } return tb.Before(ta) }}) - corpus.RUnlock() if err != nil { return nil, err }