NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[DependencyInjection] Use WeakReference to break circular references in the container#48469

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.

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

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:6.3fromnicolas-grekas:di-weak-ref

Dec 9, 2022

Merged

[DependencyInjection] Use WeakReference to break circular references in the container#48469

fabpot merged 1 commit intosymfony:6.3fromnicolas-grekas:di-weak-ref

Dec 9, 2022

Conversation

Copy link

Member

nicolas-grekas commentedDec 4, 2022

Q	A
Branch?	6.3
Bug fix?	no
New feature?	no
Deprecations?	no
Tickets	-
License	MIT
Doc PR	-

While working on#48461, I realized that we could break many circular loops involving the container by using aWeakReference to access it. This happens when we generate closures.

Conceptually, we generate this at the moment:

$internalClosure =function () {return$this->get('foo'); };

And we'd generate this with this PR:

$containerRef = \WeakReference::create($this);$internalClosure =staticfunction ()use ($containerRef) {return$containerRef->get()->get('foo'); };

This should reduce the pressure on the garbage collector.

carsonbot added Status: Needs Review DependencyInjection labels

Dec 4, 2022

carsonbot added this to the6.3 milestone

Dec 4, 2022

nicolas-grekas force-pushed thedi-weak-ref branch from65d7233 to315ae63Compare

December 4, 2022 15:34

nicolas-grekas mentioned this pull request

Dec 5, 2022

[DependencyInjection][HttpKernel] Introduce build parameters#47680

Merged

Copy link

Member

chalasr commentedDec 5, 2022

Rebase needed

nicolas-grekas force-pushed thedi-weak-ref branch from315ae63 to11a279fCompare

December 5, 2022 12:39

Copy link

MemberAuthor

nicolas-grekas commentedDec 5, 2022

Rebased

nicolas-grekas force-pushed thedi-weak-ref branch 2 times, most recently from93d9b59 to2f16657Compare

December 5, 2022 12:51

Copy link

Member

fabpot commentedDec 9, 2022

Rebase needed again :)

[DependencyInjection] Use WeakReference to break circular references …

a3b7fca

…in the container

nicolas-grekas force-pushed thedi-weak-ref branch from2f16657 toa3b7fcaCompare

December 9, 2022 07:49

Copy link

MemberAuthor

nicolas-grekas commentedDec 9, 2022

rebased (failures unrelated)

Copy link

Member

fabpot commentedDec 9, 2022

Thank you@nicolas-grekas.

fabpot merged commitf35572f intosymfony:6.3

Dec 9, 2022

stof reviewed

Dec 9, 2022

View reviewed changes

src/Symfony/Component/DependencyInjection/Container.phpShow resolvedHide resolved

src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.phpShow resolvedHide resolved

nicolas-grekas deleted the di-weak-ref branch

December 9, 2022 12:29

nicolas-grekas mentioned this pull request

Mar 14, 2023

[DependencyInjection] Use weak references in ContainerBuilder#49684

Merged

nicolas-grekas added a commit that referenced this pull request

Mar 14, 2023

minor#49684[DependencyInjection] Use weak references in ContainerBu…

cb39a7f

…ilder (nicolas-grekas)This PR was merged into the 6.3 branch.Discussion----------[DependencyInjection] Use weak references in ContainerBuilder| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Same as#48469 for ContainerBuilder.Commits-------749d4b9 [DependencyInjection] Use weak references in ContainerBuilder

nicolas-grekas mentioned this pull request

May 11, 2023

Update ci.yml with Symfony 6.2, 6.3lexik/LexikFormFilterBundle#368

Merged

HypeMC mentioned this pull request

May 25, 2023

[DependencyInjection] WeakReference on 6.3 cause error#50439

Closed

nicolas-grekas mentioned this pull request

May 26, 2023

[DependencyInjection] Revert "Use weak references in the container"#50440

Merged

nicolas-grekas added a commit that referenced this pull request

May 26, 2023

bug#50440[DependencyInjection] Revert "Use weak references in the c…

542f039

…ontainer" (nicolas-grekas)This PR was merged into the 6.3 branch.Discussion----------[DependencyInjection] Revert "Use weak references in the container"| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#50439| License       | MIT| Doc PR        | -This is the second time this issue is reported, let's revert PR#48469 and#49684 before it does more harm :)Commits-------d6ea474 [DependencyInjection] Revert "Use weak references in the container"

HypeMC mentioned this pull request

Jul 16, 2023

[DependencyInjection] Fix fetching lazy non-shared services multiple times with as filestrue#50996

Merged

nicolas-grekas added a commit that referenced this pull request

Jul 19, 2023

minor#50996[DependencyInjection] Fix fetching lazy non-shared servi…

0479bd1

…ces multiple times with as files `true` (HypeMC)This PR was merged into the 6.3 branch.Discussion----------[DependencyInjection] Fix fetching lazy non-shared services multiple times with as files `true`| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#50975| License       | MIT| Doc PR        | -Followup to#50985, fixes case when `as_files` is `true`.This particular problem exists only on v6.3 and was introduced in#48469.Commits-------cb434a4 [DependencyInjection] Fix fetching lazy non-shared services multiple times with as files true