Skip to content

fix: ensure cache does not return stale created and ttl values#1469

Merged
bdraco merged 11 commits into
masterfrom
cache_key_fix
Jan 8, 2025
Merged

fix: ensure cache does not return stale created and ttl values#1469
bdraco merged 11 commits into
masterfrom
cache_key_fix

Conversation

@bdraco

@bdraco bdraco commented Jan 8, 2025

Copy link
Copy Markdown
Member

❗ This PR corrects a long standing (3+ years) severe flaw in the cache design that was noticed while implementing #1465

The cache could return old stale records (incorrect created/ttls -- the contents of the record was correct) when the record was replaced in the underlying dict since the key is not updated (only the value is).

The following functions where affected:

cache.async_entries_with_name
cache.async_entries_with_server
cache.async_all_by_details
cache.entries_with_name
cache.entries_with_server
cache.get_all_by_details
cache.get_by_details
cache.get (never used)

Technically breaking change:
async_entries_with_name and async_entries_with_server now return a list instead of a dict which makes entries_with_name and entries_with_server. They were likely only used internally

bdraco added 2 commits January 8, 2025 11:04
This PR corrects a severe flaw in the cache design

The cache could return old stale records when the record
was replaced in the underlying dict since the key is not
updated.

The following functions where affected:

async_entries_with_name
async_entries_with_server
entries_with_name
entries_with_server
@codspeed-hq

codspeed-hq Bot commented Jan 8, 2025

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #1469 will not alter performance

Comparing cache_key_fix (baa2700) with master (ebbb2af)

Summary

✅ 3 untouched benchmarks

@bdraco bdraco changed the title fix: ensure cache always returns the values and not keys fix: ensure cache does not return stale values Jan 8, 2025
@bdraco bdraco changed the title fix: ensure cache does not return stale values fix: ensure cache does not return stale created and ttl values Jan 8, 2025
@bdraco bdraco marked this pull request as ready for review January 8, 2025 21:42
@codecov

This comment was marked as outdated.

@bdraco bdraco merged commit e05055c into master Jan 8, 2025
@bdraco bdraco deleted the cache_key_fix branch January 8, 2025 21:48
bdraco added a commit to home-assistant/core that referenced this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant