Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Dec 9, 2022

Conversation

nicolas-grekas
Copy link
Member

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 a WeakReference 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 = static function () use ($containerRef) { return $containerRef->get()->get('foo'); };

This should reduce the pressure on the garbage collector.

@chalasr
Copy link
Member

chalasr commented Dec 5, 2022

Rebase needed

@nicolas-grekas
Copy link
Member Author

Rebased

@nicolas-grekas nicolas-grekas force-pushed the di-weak-ref branch 2 times, most recently from 93d9b59 to 2f16657 Compare December 5, 2022 12:51
@fabpot
Copy link
Member

fabpot commented Dec 9, 2022

Rebase needed again :)

@nicolas-grekas
Copy link
Member Author

rebased (failures unrelated)

@fabpot
Copy link
Member

fabpot commented Dec 9, 2022

Thank you @nicolas-grekas.

@fabpot fabpot merged commit f35572f into symfony:6.3 Dec 9, 2022
@nicolas-grekas nicolas-grekas deleted the di-weak-ref branch December 9, 2022 12:29
nicolas-grekas added a commit that referenced this pull request Mar 14, 2023
…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 added a commit that referenced this pull request May 26, 2023
…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"
nicolas-grekas added a commit that referenced this pull request Jul 19, 2023
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants