Skip to content

[GH-8410] Fix memory leak in new toIterable and state bug.#8467

Merged
beberlei merged 6 commits intodoctrine:2.8.xfrom
beberlei:GH-8410-FixMemoryLeakToIterable
Feb 16, 2021
Merged

[GH-8410] Fix memory leak in new toIterable and state bug.#8467
beberlei merged 6 commits intodoctrine:2.8.xfrom
beberlei:GH-8410-FixMemoryLeakToIterable

Conversation

@beberlei
Copy link
Copy Markdown
Member

@beberlei beberlei commented Feb 8, 2021

The new AbstractQuery::toIterable() had a memory leak that AbstractQuery::iterable() did not have. This leak is now fixed.

After fixing the leak, one test failed where the identity map in ObjectHydrator triggered and lead to a notice. Introduced a new AbstractHydrator::cleanupAfterRowIteration() that the ObjectHydrator uses to cleanup the state.

When the result contains multiple entities, there must be special handling, otherwise only the last entity is returned.

In addition mixed result queries with scalar mappings don't work with iteration and are now explicitly prevented by exception.

Fixes #8410 Fixes #8413 Fixes #8387
Supersedes #8412 #8414

@beberlei beberlei added this to the 2.8.2 milestone Feb 8, 2021
simPod
simPod previously approved these changes Feb 8, 2021
snapshotpl
snapshotpl previously approved these changes Feb 8, 2021
The new AbstractQuery::toIterable() had a memory leak that
AbstractQuery::iterable() did not have. This leak is now fixed.

After fixing the leak, one test failed where the identity map in
ObjectHydrator triggered and lead to a notice. Introduced a new
AbstractHydrator::cleanupAfterRowIteration() that the ObjectHydrator
uses to cleanup the state.
When multiple entity results are part of a row, the result handling
must be different. In addition mixed results with scalars are broken
and now throw an exception as illegal operation.
@beberlei beberlei dismissed stale reviews from snapshotpl and simPod via b5f740c February 16, 2021 15:15
@beberlei beberlei force-pushed the GH-8410-FixMemoryLeakToIterable branch from 094453c to b5f740c Compare February 16, 2021 15:15
@beberlei beberlei merged commit 30a7c2a into doctrine:2.8.x Feb 16, 2021
@beberlei beberlei deleted the GH-8410-FixMemoryLeakToIterable branch February 16, 2021 16:52
@alex-dev
Copy link
Copy Markdown

In addition mixed result queries with scalar mappings don't work with iteration and are now explicitly prevented by exception.

Do we have a follow-up fix for this or an issue tracking it?

@kbond
Copy link
Copy Markdown
Contributor

kbond commented Mar 1, 2021

I created an issue (#8520) to at least track the problem this PR introduced.

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.

Issue with toIterable() and selecting multiple entities Batch Processing using toIterable() Query::iterate not marked as deprecated

5 participants