Skip to content

Drop entry acl#6

Open
ewgRa wants to merge 6 commits into
symfony:mainfrom
ewgRa:drop_entry_acl
Open

Drop entry acl#6
ewgRa wants to merge 6 commits into
symfony:mainfrom
ewgRa:drop_entry_acl

Conversation

@ewgRa

@ewgRa ewgRa commented Sep 10, 2015

Copy link
Copy Markdown
Contributor
Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? -
Fixed tickets symfony/symfony#2376
License MIT
Doc PR

As I notice in symfony/symfony#2376, Acl have memory leak.

This is kind of bad design of code, that use something like this:
$this->entry = new Entry(this);

as a result Acl have refcount=2, and second refcount from object itself. As a result when we do unset($acl) it is not free memory at all. This PR - fix for this.

@linaori

linaori commented Sep 11, 2015

Copy link
Copy Markdown
Contributor

Can you add an UPGRADE-3.0.md file which states exactly which changes are done that break backwards compatibility? This will help people upgrade.

Example: https://github.com/symfony/symfony/blob/master/UPGRADE-3.0.md

@ewgRa

ewgRa commented Sep 14, 2015

Copy link
Copy Markdown
Contributor Author

done.
Also fix another places in code and add test for releaseMemory. Review please

@ewgRa

ewgRa commented Sep 14, 2015

Copy link
Copy Markdown
Contributor Author

@iltar ClassUtils not related to this PR. It was just local quick fix for test run that I accidentally commit and then revert it back. Problem with ClassUtils must be fixed in PR #3

@ewgRa

ewgRa commented Sep 14, 2015

Copy link
Copy Markdown
Contributor Author

2.8 tests are broken now, due to ClassUtils. When I merge your PR about ClassUtils to my PR, all tests passed.

@nicolas-grekas nicolas-grekas changed the base branch from master to main November 19, 2020 12:30
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.

2 participants