pkg/index: make corpus RLock take a context

Also remove extra (deadlocking) RLock in pkg/search.

Updates issue #709

Change-Id: I556a1fbf9217f482b6a51e74c28a019dea8369a2
This commit is contained in:
mpl 2016-04-21 11:14:07 -07:00
parent c2e4fc7de6
commit 8580b811cf
4 changed files with 20 additions and 16 deletions

View File

@ -44,7 +44,7 @@ import (
// Corpus is an in-memory summary of all of a user's blobs' metadata. // Corpus is an in-memory summary of all of a user's blobs' metadata.
type Corpus struct { type Corpus struct {
mu sync.RWMutex mu sync.RWMutex
//mu syncutil.RWMutexTracker // when debugging // mu syncdebug.RWMutexTracker // when debugging
// building is true at start while scanning all rows in the // building is true at start while scanning all rows in the
// index. While building, certain invariants (like things // 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. // 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. // RUnlock unlocks the Corpus for reads.
func (c *Corpus) RUnlock() { c.mu.RUnlock() } func (c *Corpus) RUnlock() { c.mu.RUnlock() }
// IsDeleted reports whether the provided blobref (of a permanode or claim) should be considered deleted. // IsDeleted reports whether the provided blobref (of a permanode or claim) should be considered deleted.
func (c *Corpus) IsDeleted(br blob.Ref) bool { func (c *Corpus) IsDeleted(ctx context.Context, br blob.Ref) bool {
c.RLock() c.RLock(ctx)
defer c.RUnlock() defer c.RUnlock()
return c.IsDeletedLocked(br) return c.IsDeletedLocked(br)
} }

View File

@ -129,8 +129,9 @@ func TestCorpusAppendPermanodeAttrValues(t *testing.T) {
func TestCorpusPermanodeAttrValueLocked(t *testing.T) { func TestCorpusPermanodeAttrValueLocked(t *testing.T) {
c, pn := newTestCorpusWithPermanode() c, pn := newTestCorpusWithPermanode()
ctx := context.TODO()
c.RLock() c.RLock(ctx)
defer c.RUnlock() defer c.RUnlock()
tests := []struct { tests := []struct {
@ -174,7 +175,8 @@ func TestCorpusPermanodeAttrValueLocked(t *testing.T) {
func TestCorpusPermanodeHasAttrValueLocked(t *testing.T) { func TestCorpusPermanodeHasAttrValueLocked(t *testing.T) {
c, pn := newTestCorpusWithPermanode() c, pn := newTestCorpusWithPermanode()
c.RLock() ctx := context.TODO()
c.RLock(ctx)
defer c.RUnlock() defer c.RUnlock()
tests := []struct { tests := []struct {
@ -294,7 +296,8 @@ func testDeletePermanodes(t *testing.T,
ch := make(chan camtypes.BlobMeta, 10) ch := make(chan camtypes.BlobMeta, 10)
var got []camtypes.BlobMeta var got []camtypes.BlobMeta
errc := make(chan error, 1) errc := make(chan error, 1)
c.RLock() ctx := context.TODO()
c.RLock(ctx)
go func() { errc <- enumFunc(c, context.TODO(), ch) }() go func() { errc <- enumFunc(c, context.TODO(), ch) }()
for blobMeta := range ch { for blobMeta := range ch {
got = append(got, blobMeta) got = append(got, blobMeta)
@ -325,7 +328,7 @@ func testDeletePermanodes(t *testing.T,
want = []blob.Ref{foopn} want = []blob.Ref{foopn}
got = got[:0] got = got[:0]
ch = make(chan camtypes.BlobMeta, 10) ch = make(chan camtypes.BlobMeta, 10)
c.RLock() c.RLock(ctx)
go func() { errc <- enumFunc(c, context.TODO(), ch) }() go func() { errc <- enumFunc(c, context.TODO(), ch) }()
for blobMeta := range ch { for blobMeta := range ch {
got = append(got, blobMeta) got = append(got, blobMeta)
@ -347,7 +350,7 @@ func testDeletePermanodes(t *testing.T,
want = []blob.Ref{foopn, bazpn} want = []blob.Ref{foopn, bazpn}
got = got[:0] got = got[:0]
ch = make(chan camtypes.BlobMeta, 10) ch = make(chan camtypes.BlobMeta, 10)
c.RLock() c.RLock(ctx)
go func() { errc <- enumFunc(c, context.TODO(), ch) }() go func() { errc <- enumFunc(c, context.TODO(), ch) }()
for blobMeta := range ch { for blobMeta := range ch {
got = append(got, blobMeta) got = append(got, blobMeta)
@ -432,7 +435,8 @@ func testEnumerateOrder(t *testing.T,
ch := make(chan camtypes.BlobMeta, 10) ch := make(chan camtypes.BlobMeta, 10)
var got []camtypes.BlobMeta var got []camtypes.BlobMeta
errc := make(chan error, 1) errc := make(chan error, 1)
c.RLock() ctx := context.TODO()
c.RLock(ctx)
go func() { errc <- enumFunc(c, context.TODO(), ch) }() go func() { errc <- enumFunc(c, context.TODO(), ch) }()
for blobMeta := range ch { for blobMeta := range ch {
got = append(got, blobMeta) got = append(got, blobMeta)
@ -488,11 +492,12 @@ func testCacheSortedPermanodesRace(t *testing.T,
} }
donec <- struct{}{} donec <- struct{}{}
}() }()
ctx := context.TODO()
go func() { go func() {
for i := 0; i < 10; i++ { for i := 0; i < 10; i++ {
ch := make(chan camtypes.BlobMeta, 10) ch := make(chan camtypes.BlobMeta, 10)
errc := make(chan error, 1) errc := make(chan error, 1)
c.RLock() c.RLock(ctx)
go func() { errc <- enumFunc(c, context.TODO(), ch) }() go func() { errc <- enumFunc(c, context.TODO(), ch) }()
for range ch { for range ch {
} }
@ -528,7 +533,8 @@ func TestLazySortedPermanodes(t *testing.T) {
enum := func(reverse bool) { enum := func(reverse bool) {
ch := make(chan camtypes.BlobMeta, 10) ch := make(chan camtypes.BlobMeta, 10)
errc := make(chan error, 1) errc := make(chan error, 1)
c.RLock() ctx := context.TODO()
c.RLock(ctx)
go func() { errc <- c.EnumeratePermanodesCreatedLocked(context.TODO(), ch, reverse) }() go func() { errc <- c.EnumeratePermanodesCreatedLocked(context.TODO(), ch, reverse) }()
for range ch { for range ch {
} }

View File

@ -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. // 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) { func (x *Index) EnumerateBlobMeta(ctx context.Context, ch chan<- camtypes.BlobMeta) (err error) {
if x.corpus != nil { if x.corpus != nil {
x.corpus.RLock() x.corpus.RLock(ctx)
defer x.corpus.RUnlock() defer x.corpus.RUnlock()
return x.corpus.EnumerateBlobMetaLocked(ctx, ch) return x.corpus.EnumerateBlobMetaLocked(ctx, ch)
} }

View File

@ -857,7 +857,7 @@ func (h *Handler) Query(rawq *SearchQuery) (*SearchResult, error) {
corpus := h.corpus corpus := h.corpus
var unlockOnce sync.Once var unlockOnce sync.Once
if corpus != nil { if corpus != nil {
corpus.RLock() corpus.RLock(ctx)
defer unlockOnce.Do(corpus.RUnlock) 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") return nil, errors.New("TODO: Sorting without a corpus unsupported")
} }
var err error var err error
corpus.RLock()
sort.Sort(sortSearchResultBlobs{res.Blobs, func(a, b *SearchResultBlob) bool { sort.Sort(sortSearchResultBlobs{res.Blobs, func(a, b *SearchResultBlob) bool {
if err != nil { if err != nil {
return false return false
@ -963,7 +962,6 @@ func (h *Handler) Query(rawq *SearchQuery) (*SearchResult, error) {
} }
return tb.Before(ta) return tb.Before(ta)
}}) }})
corpus.RUnlock()
if err != nil { if err != nil {
return nil, err return nil, err
} }