dbal icon indicating copy to clipboard operation
dbal copied to clipboard

[Bug] Query Cache mangled if saved by-reference

Open Slamdunk opened this issue 1 year ago • 3 comments

Bug emerged in https://github.com/doctrine/dbal/pull/6510#discussion_r1804475987

The current implementation of query cache relies on the cache not saved by-reference; the bug has never been seen before because by default new ArrayAdapter() saves the cache with serialization, hence breaking the by-reference pointer.

Once the by-reference tecnique is used, two issues pop up:

  1. \Doctrine\DBAL\Cache\ArrayResult::$num is never reset, so once it gets incremented in the first \Doctrine\DBAL\Cache\ArrayResult::fetch call, the following calls will always fail
  2. Even considering fixing the $num property reset, a manual call on \Doctrine\DBAL\Result::free will by cascade call the \Doctrine\DBAL\Cache\ArrayResult::free method erasing all the saved results

I think that the ArrayResult implementation is not the culprit, but rather the #6510 giving to the cache backend the internal object by reference instead of giving it a copy.

Slamdunk avatar Oct 17 '24 12:10 Slamdunk

We can probably solve this by cloning the ArrayResult before storing and returning it. That being said, you really should not use this kind of cache in production.

derrabus avatar Oct 17 '24 12:10 derrabus

That bein said, you really should not use this kind of cache in production.

I disagree: we have in production many long-running processes where this kind of cache is enough and more performant than the alternatives

Slamdunk avatar Oct 17 '24 12:10 Slamdunk

I mean, you do you, but you're like asking for the kind of trouble that you see here. No other cache would behave this way. What you call "the by-reference tecnique" is simply put non-standard behavior.

derrabus avatar Oct 17 '24 13:10 derrabus

Thank you.

derrabus avatar Oct 21 '24 07:10 derrabus