Skip to content
This repository has been archived by the owner on Feb 12, 2019. It is now read-only.

prefetcher: avoid mem leaks by not storing block in request #1988

Merged
merged 3 commits into from
Dec 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions libkbfs/block_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ func (cb *CommonBlock) NewEmpty() Block {
return NewCommonBlock()
}

// NewEmptier implements the Block interface for CommonBlock.
func (cb *CommonBlock) NewEmptier() func() Block {
return NewCommonBlock
}

// ToCommonBlock implements the Block interface for CommonBlock.
func (cb *CommonBlock) ToCommonBlock() *CommonBlock {
return cb
Expand Down Expand Up @@ -226,6 +231,11 @@ func (db *DirBlock) NewEmpty() Block {
return NewDirBlock()
}

// NewEmptier implements the Block interface for DirBlock.
func (db *DirBlock) NewEmptier() func() Block {
return NewDirBlock
}

// IsTail implements the Block interface for DirBlock.
func (db *DirBlock) IsTail() bool {
if db.IsInd {
Expand Down Expand Up @@ -449,6 +459,11 @@ func (fb *FileBlock) NewEmpty() Block {
return &FileBlock{}
}

// NewEmptier implements the Block interface for FileBlock.
func (fb *FileBlock) NewEmptier() func() Block {
return NewFileBlock
}

// IsTail implements the Block interface for FileBlock.
func (fb *FileBlock) IsTail() bool {
if fb.IsInd {
Expand Down
8 changes: 8 additions & 0 deletions libkbfs/crypto_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ func (tb TestBlock) NewEmpty() Block {
return &TestBlock{}
}

func (tb TestBlock) NewEmptier() func() Block {
return tb.NewEmpty
}

func (tb *TestBlock) Set(other Block) {
otherTb := other.(*TestBlock)
tb.A = otherTb.A
Expand Down Expand Up @@ -396,6 +400,10 @@ func (tba testBlockArray) NewEmpty() Block {
return &testBlockArray{}
}

func (tba testBlockArray) NewEmptier() func() Block {
return tba.NewEmpty
}

func (tba testBlockArray) ToCommonBlock() *CommonBlock {
return nil
}
Expand Down
3 changes: 3 additions & 0 deletions libkbfs/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ type Block interface {
SetEncodedSize(size uint32)
// NewEmpty returns a new block of the same type as this block
NewEmpty() Block
// NewEmptier returns a function that creates a new block of the
// same type as this block.
NewEmptier() func() Block
// Set sets this block to the same value as the passed-in block
Set(other Block)
// ToCommonBlock retrieves this block as a *CommonBlock.
Expand Down
28 changes: 28 additions & 0 deletions libkbfs/mocks_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions libkbfs/prefetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type prefetcherConfig interface {

type prefetchRequest struct {
ptr BlockPointer
block Block
newBlock func() Block
kmd KeyMetadata
priority int
lifetime BlockCacheLifetime
Expand Down Expand Up @@ -264,7 +264,7 @@ func (p *blockPrefetcher) completePrefetch(
p.clearRescheduleState(blockID)
delete(p.rescheduled, blockID)
defer pp.Close()
b := pp.req.block.NewEmpty()
b := pp.req.newBlock()
err := <-p.retriever.Request(
pp.ctx, defaultOnDemandRequestPriority, pp.req.kmd, pp.req.ptr,
b, pp.req.lifetime, BlockRequestSolo)
Expand Down Expand Up @@ -360,8 +360,8 @@ func (p *blockPrefetcher) request(ctx context.Context, priority int,
if !isPrefetchWaiting {
// If the block isn't in the tree, we add it with a block count of 1 (a
// later TriggerPrefetch will come in and decrement it).
req := &prefetchRequest{ptr, block, kmd, priority, lifetime,
NoPrefetch, action, nil}
req := &prefetchRequest{ptr, block.NewEmptier(), kmd, priority,
lifetime, NoPrefetch, action, nil}
pre = p.newPrefetch(1, false, req)
p.prefetches[ptr.ID] = pre
}
Expand All @@ -375,7 +375,7 @@ func (p *blockPrefetcher) request(ctx context.Context, priority int,
// handled.
pre.req.action = newAction
ch := p.retriever.Request(
pre.ctx, priority, kmd, ptr, block, lifetime, action)
pre.ctx, priority, kmd, ptr, block.NewEmpty(), lifetime, action)
p.inFlightFetches.In() <- ch
}
_, isParentWaiting := p.prefetches[parentPtr.ID]
Expand Down Expand Up @@ -726,7 +726,7 @@ func (p *blockPrefetcher) run(testSyncCh <-chan struct{}) {
req.ptr, req.action)

// Ensure the block is in the right cache.
b := req.block.NewEmpty()
b := req.newBlock()
err := <-p.retriever.Request(
ctx, defaultOnDemandRequestPriority, req.kmd,
req.ptr, b, req.lifetime, req.action.SoloAction())
Expand Down Expand Up @@ -971,7 +971,7 @@ func (p *blockPrefetcher) ProcessBlockForPrefetch(ctx context.Context,
ptr BlockPointer, block Block, kmd KeyMetadata, priority int,
lifetime BlockCacheLifetime, prefetchStatus PrefetchStatus,
action BlockRequestAction) {
req := &prefetchRequest{ptr, block.NewEmpty(), kmd, priority, lifetime,
req := &prefetchRequest{ptr, block.NewEmptier(), kmd, priority, lifetime,
prefetchStatus, action, nil}
if prefetchStatus == FinishedPrefetch {
// Finished prefetches can always be short circuited.
Expand Down