Skip to content

[Cache] ArrayAdapter serialization exception clean $expiries #60122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Apr 7, 2025
Merged

[Cache] ArrayAdapter serialization exception clean $expiries #60122

merged 2 commits into from
Apr 7, 2025
+14 −1

Conversation

bastien-wink
Copy link
Contributor

@bastien-wink bastien-wink commented Apr 2, 2025 •
edited by nicolas-grekas
Loading

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #60121
License MIT

In ArrayAdapter.php:339, if $value contains a Closure, an Exception is triggered and $this->values get cleaned.

  try {
      $serialized = serialize($value);
  } catch (\Exception $e) {
      unset($this->values[$key], $this->expiries[$key], $this->tags[$key]);
      $type = get_debug_type($value);
      $message = \sprintf('Failed to save key "{key}" of type %s: %s', $type, $e->getMessage());
      CacheItem::log($this->logger, $message, ['key' => $key, 'exception' => $e, 'cache-adapter' => get_debug_type($this)]);

      return null;
  }

$this->expiries is used in ArrayAdapter.php:300 to determine cache hit. If hash is found in $this->expiries but missing in $this->keys it generate a NOTICE.

On serialize($value); catch Exception, clean expiries with unset.

@carsonbot carsonbot added this to the 7.3 milestone Apr 2, 2025
@carsonbot carsonbot changed the title bug #60121[Cache] ArrayAdapter serialization exception clean $expiries [Cache] bug #60121 ArrayAdapter serialization exception clean $expiries Apr 2, 2025
@stof
Copy link
Member

stof commented Apr 2, 2025

Please add a test to prevent regressions

@nicolas-grekas nicolas-grekas changed the title [Cache] bug #60121 ArrayAdapter serialization exception clean $expiries [Cache] ArrayAdapter serialization exception clean $expiries Apr 4, 2025
@nicolas-grekas nicolas-grekas modified the milestones: 7.3, 6.4 Apr 4, 2025
@nicolas-grekas nicolas-grekas changed the base branch from 7.3 to 6.4 April 4, 2025 08:20
@nicolas-grekas nicolas-grekas changed the base branch from 6.4 to 7.3 April 4, 2025 08:20
@nicolas-grekas nicolas-grekas changed the base branch from 7.3 to 6.4 April 7, 2025 19:58
@nicolas-grekas
Copy link
Member

Thank you @bastien-wink.

@nicolas-grekas nicolas-grekas merged commit 196c99e into symfony:6.4 Apr 7, 2025
8 of 11 checks passed
@nicolas-grekas nicolas-grekas mentioned this pull request Apr 7, 2025
Merged
nicolas-grekas added a commit that referenced this pull request Apr 8, 2025
@nicolas-grekas
bug #60172 [Cache] Fix invalidating on save failures with Array|ApcuA…
06f53ef 
…dapter (nicolas-grekas)

This PR was merged into the 6.4 branch.

Discussion
----------

[Cache] Fix invalidating on save failures with Array|ApcuAdapter

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

I merged #60122 and then realized it was fixing a bad behavior. Saving an invalid key should not remove the previous one, if any.

Commits
-------

30640b2 [Cache] Fix invalidating on save failures with Array|ApcuAdapter
This was referenced May 2, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lyrixx lyrixx Awaiting requested review from lyrixx

@yceruto yceruto Awaiting requested review from yceruto

@chalasr chalasr Awaiting requested review from chalasr

@dunglas dunglas Awaiting requested review from dunglas

@OskarStark OskarStark Awaiting requested review from OskarStark

@jderusse jderusse Awaiting requested review from jderusse

@xabbuh xabbuh Awaiting requested review from xabbuh

Assignees
No one assigned
Labels
Bug Cache Status: Needs Review
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.

[Cache] ArrayAdapter serialization exception does not clean $expiries
4 participants
@bastien-wink @stof @nicolas-grekas @carsonbot