Skip to content

Commit

Permalink
Fix accessing the Etag causing panic in Download method of blockcache
Browse files Browse the repository at this point in the history
  • Loading branch information
syeleti-msft committed Feb 6, 2025
1 parent 09526c0 commit effd80c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
11 changes: 7 additions & 4 deletions component/block_cache/block_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -938,12 +938,18 @@ func (bc *BlockCache) refreshBlock(handle *handlemap.Handle, index uint64, prefe

// lineupDownload : Create a work item and schedule the download
func (bc *BlockCache) lineupDownload(handle *handlemap.Handle, block *Block, prefetch bool) {
etagInterface, found := handle.GetValue("ETAG")
etag := ""
if found {
etag = etagInterface.(string)
}
item := &workItem{
handle: handle,
block: block,
prefetch: prefetch,
failCnt: 0,
upload: false,
ETag: etag,
}

// Remove this block from free block list and add to in-process list
Expand Down Expand Up @@ -1055,10 +1061,7 @@ func (bc *BlockCache) download(item *workItem) {

// Compare the ETAG value and fail download if blob has changed
if etag != "" {
item.handle.Lock()
etagVal, found := item.handle.GetValue("ETAG")
item.handle.Unlock()
if found && etagVal != etag {
if item.ETag != "" && item.ETag != etag {
log.Err("BlockCache::download : Blob has changed for %v=>%s (index %v, offset %v)", item.handle.ID, item.handle.Path, item.block.id, item.block.offset)
item.block.Failed()
item.block.Ready(BlockStatusDownloadFailed)
Expand Down
10 changes: 10 additions & 0 deletions component/block_cache/threadpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,18 @@ type workItem struct {
failCnt int32 // How many times this item has failed to download
upload bool // Flag marking this is a upload request or not
blockId string // BlockId of the block
ETag string // Etag of the file before scheduling.
}

// Reason for storing Etag in workitem struct
// here getting the value of ETag inside upload/download methods
// from the handle is somewhat tricker.
// firstly we need to aquire a lock to read it from the handle.

Check failure on line 78 in component/block_cache/threadpool.go

View workflow job for this annotation

GitHub Actions / Check for spelling errors

aquire ==> acquire
// In these methods the handle may be locked/maynotbe locked by
// other go routine hence acquring would cause a deadlock.

Check failure on line 80 in component/block_cache/threadpool.go

View workflow job for this annotation

GitHub Actions / Check for spelling errors

acquring ==> acquiring
// It is already locked if the call came from the readInBuffer.
// It is may be locked if the call come from the prefetch.

// newThreadPool creates a new thread pool
func newThreadPool(count uint32, reader func(*workItem), writer func(*workItem)) *ThreadPool {
if count == 0 || reader == nil {
Expand Down
4 changes: 3 additions & 1 deletion test/e2e_tests/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ func (suite *fileTestSuite) TestOpenFlag_O_TRUNC() {
fileName := suite.testPath + "/test_on_open"
buf := "foo"
srcFile, err := os.OpenFile(fileName, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0666)
srcFile.Write([]byte(buf))
suite.Nil(err)
bytesWritten, err := srcFile.Write([]byte(buf))
suite.Equal(len(buf), bytesWritten)
suite.Nil(err)
srcFile.Close()

Expand Down

0 comments on commit effd80c

Please sign in to comment.