From d5cdf3736971a94722592ce497f752d16f313d4f Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Thu, 28 Sep 2023 18:50:15 -0700 Subject: [PATCH] Lock updating info in lazyItem (#4401) Don't allow concurrent reads/writes to info in lazyItem so we can make stronger assumptions about state --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [x] :green_heart: E2E --- src/internal/data/item.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/internal/data/item.go b/src/internal/data/item.go index 2403e63aa..862699a5d 100644 --- a/src/internal/data/item.go +++ b/src/internal/data/item.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "io" + "sync" "time" "github.com/alcionai/clues" @@ -110,6 +111,7 @@ func NewLazyItem( // made. type lazyItem struct { ctx context.Context + mu sync.Mutex id string errs *fault.Bus itemGetter ItemDataGetter @@ -127,12 +129,18 @@ type lazyItem struct { delInFlight bool } -func (i lazyItem) ID() string { +func (i *lazyItem) ID() string { return i.id } func (i *lazyItem) ToReader() io.ReadCloser { return lazy.NewLazyReadCloser(func() (io.ReadCloser, error) { + // Don't allow getting Item info while trying to initialize said info. + // GetData could be a long running call, but in theory nothing should happen + // with the item until a reader is returned anyway. + i.mu.Lock() + defer i.mu.Unlock() + reader, info, delInFlight, err := i.itemGetter.GetData(i.ctx, i.errs) if err != nil { return nil, clues.Stack(err) @@ -159,11 +167,14 @@ func (i *lazyItem) ToReader() io.ReadCloser { }) } -func (i lazyItem) Deleted() bool { +func (i *lazyItem) Deleted() bool { return false } -func (i lazyItem) Info() (details.ItemInfo, error) { +func (i *lazyItem) Info() (details.ItemInfo, error) { + i.mu.Lock() + defer i.mu.Unlock() + if i.delInFlight { return details.ItemInfo{}, clues.Stack(ErrNotFound).WithClues(i.ctx) } else if i.info == nil { @@ -174,6 +185,6 @@ func (i lazyItem) Info() (details.ItemInfo, error) { return *i.info, nil } -func (i lazyItem) ModTime() time.Time { +func (i *lazyItem) ModTime() time.Time { return i.modTime }