Skip to content

Commit

Permalink
Fix race condition in PassthroughProgramCache.
Browse files Browse the repository at this point in the history
PassthroughProgramCache has code to deal with multiple threads,
but BlobCacheGet() does not hold the lock for the entire time
it is called. This means it's possible for BlobCacheGet() to
attempt to use a cache value that was deleted by another thread.
BlobCacheGet() needs to hold the lock for the entirety of the
function.

Add a new PassthroughProgramCache::GetSize() method to handle the
locking since BlobCacheGet() is a static method which can not hold lock.

Bug: 1464739
Change-Id: Ibdbc78823fbf0032b8f0d84f0aafa249f93bf3d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4683916
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: vikas soni <vikassoni@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Auto-Submit: vikas soni <vikassoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1170276}
  • Loading branch information
vikaschromie authored and Chromium LUCI CQ committed Jul 14, 2023
1 parent a77d713 commit 252897c
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 21 deletions.
56 changes: 35 additions & 21 deletions gpu/command_buffer/service/passthrough_program_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,44 @@ void PassthroughProgramCache::Set(Key&& key, Value&& value) {

const PassthroughProgramCache::ProgramCacheValue* PassthroughProgramCache::Get(
const Key& key) {
base::AutoLock auto_lock(lock_);
lock_.AssertAcquired();
ProgramLRUCache::iterator found = store_.Get(key);
return found == store_.end() ? nullptr : &found->second;
}

EGLsizeiANDROID PassthroughProgramCache::BlobCacheGetImpl(
const void* key,
EGLsizeiANDROID key_size,
void* value,
EGLsizeiANDROID value_size) {
if (key_size < 0) {
return 0;
}

const uint8_t* key_begin = reinterpret_cast<const uint8_t*>(key);
PassthroughProgramCache::Key entry_key(key_begin, key_begin + key_size);

// Note that the |lock_| should be held during whole time ProgramCacheValue is
// being accessed below.
base::AutoLock auto_lock(lock_);
const PassthroughProgramCache::ProgramCacheValue* cache_value =
g_program_cache->Get(std::move(entry_key));

if (!cache_value) {
return 0;
}

const PassthroughProgramCache::Value& entry_value = cache_value->data();

if (value_size > 0) {
if (static_cast<size_t>(value_size) >= entry_value.size()) {
memcpy(value, entry_value.data(), entry_value.size());
}
}

return entry_value.size();
}

void PassthroughProgramCache::BlobCacheSet(const void* key,
EGLsizeiANDROID key_size,
const void* value,
Expand Down Expand Up @@ -211,26 +244,7 @@ EGLsizeiANDROID PassthroughProgramCache::BlobCacheGet(
if (!g_program_cache)
return 0;

if (key_size < 0)
return 0;

const uint8_t* key_begin = reinterpret_cast<const uint8_t*>(key);
PassthroughProgramCache::Key entry_key(key_begin, key_begin + key_size);

const PassthroughProgramCache::ProgramCacheValue* cacheValue =
g_program_cache->Get(std::move(entry_key));

if (!cacheValue)
return 0;

const PassthroughProgramCache::Value& entry_value = cacheValue->data();

if (value_size > 0) {
if (static_cast<size_t>(value_size) >= entry_value.size())
memcpy(value, entry_value.data(), entry_value.size());
}

return entry_value.size();
return g_program_cache->BlobCacheGetImpl(key, key_size, value, value_size);
}

PassthroughProgramCache::ProgramCacheValue::ProgramCacheValue(
Expand Down
4 changes: 4 additions & 0 deletions gpu/command_buffer/service/passthrough_program_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ class GPU_GLES2_EXPORT PassthroughProgramCache : public ProgramCache {
bool CacheEnabled() const;

const ProgramCacheValue* Get(const Key& key);
EGLsizeiANDROID BlobCacheGetImpl(const void* key,
EGLsizeiANDROID key_size,
void* value,
EGLsizeiANDROID value_size);

friend class ProgramCacheValue;

Expand Down

0 comments on commit 252897c

Please sign in to comment.