T16976 findfirst segfault memory cache limit#16988
Conversation
- Add destructor to PdoResult to close PDOStatement and nullify resources, preventing segmentation faults. - Introduce cache eviction mechanism in Query to limit memory growth during high-iteration loops. - Add tests for PdoResult destructor and Query cache eviction to ensure proper resource management. Assisted-by: GitHub Copilot
…y growth in high-iteration loops Assisted-by: GitHub Copilot
|
@Lucifer07 I apologize I did not post in our documentation early enough regarding AI development so you did not know. I would kindly ask you to redo this PR using instructions from here: https://docs.phalcon.io/5.12/ai-development/ In short, every commit assisted by an AI has to have it in the commit message. Apologies again. |
Lucifer07
force-pushed
the
T16976-findfirst-segfault-memory-cache-limit
branch
from
14f596f to
cb376fd
Compare
| try { | ||
| this->pdoStatement->closeCursor(); | ||
| } catch \Exception { | ||
| } |
There was a problem hiding this comment.
There has to be some code in the catch branch to alert the developer that an exception has happened. The way this is now, it will not only work in this case (fixing this bug) but it will also suppress any other exception that might happen unrelated to this fix.
| null, | ||
| true | ||
| ); | ||
| } |
There was a problem hiding this comment.
Please add the 1024 and 512 as constants in this file with a docblock of what they represent
There was a problem hiding this comment.
I would change that to be 75% instead of 50%. It does reduce the internal cache but not halving it which could in theory have a bigger impact.
| use Phalcon\Tests\Support\Traits\DiTrait; | ||
|
|
||
| /** | ||
| * @issue https://github.com/phalcon/cphalcon/issues/16955 |
There was a problem hiding this comment.
This one goes in the test i.e. method
| /** | ||
| * Tests that PdoResult::__destruct calls closeCursor and nullifies resources. | ||
| * | ||
| * @author Phalcon Team <team@phalcon.io> |
There was a problem hiding this comment.
This one is your name - we didn't do this work.
There was a problem hiding this comment.
Same with the rest of the tests in this file
2 participants
Hello!
In raising this pull request, I confirm the following:
Small description of change:
Running findFirst in a high-iteration loop (e.g. 5M iterations) causes a segmentation fault instead of a standard PHP Allowed memory size exhausted error. Two fixes applied:
Thanks