Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add DI specification test for ensuring externally provided instance services are not disposed on provider disposal. #104853

Closed
wants to merge 6 commits into from

Conversation

alistairjevans
Copy link
Contributor

Fixes #102651

…ervices are not disposed when the provider is disposed.
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 13, 2024
@alistairjevans
Copy link
Contributor Author

Looks like the external container tests are failing, which they will until we fix the behaviour I'm testing for in Autofac! What's the approach here, fix Autofac before merging, and update the external tests project? Or mark the new test as skipped for Autofac to get this merged?

@steveharter
Copy link
Member

Looks like the external container tests are failing, which they will until we fix the behaviour I'm testing for in Autofac! What's the approach here, fix Autofac before merging, and update the external tests project? Or mark the new test as skipped for Autofac to get this merged?

It depends on timing. If the Autofac fix is coming soon, I'd wait and then update the version in the Microsoft.Extensions.DependencyInjection.ExternalContainers.Tests project:

    <PackageReference Include="Autofac.Extensions.DependencyInjection" Version="8.0.0" />

If it will be a while, then use

[ActiveIssue("https://github.com/dotnet/runtime/issues/102651")]

on the test.

@alistairjevans
Copy link
Contributor Author

We'll get an Autofac release out with the fix first; shouldn't be long.

@steveharter steveharter added this to the 10.0.0 milestone Aug 14, 2024
@steveharter steveharter added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 14, 2024
@alistairjevans
Copy link
Contributor Author

@steveharter, the updated package is up, 10.0.0, and I've updated the project. Some of the checks claim not to be able to find the new version though. Anything needs to be updated or cache flushed?

@steveharter steveharter removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 25, 2024
@steveharter
Copy link
Member

@steveharter, the updated package is up, 10.0.0, and I've updated the project. Some of the checks claim not to be able to find the new version though. Anything needs to be updated or cache flushed?

@alistairjevans can you sync with main please. Autofac was updated to 10 recently here: #107639. Thanks

# Conflicts:
#	src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.External.Tests/Microsoft.Extensions.DependencyInjection.ExternalContainers.Tests.csproj
@steveharter steveharter added the needs-author-action An issue or pull request that requires more info or actions from the author. label Dec 4, 2024
@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Dec 5, 2024
@steveharter
Copy link
Member

Note after the recent merge from main, this PR only has the new test ServiceInstanceAreNotDisposedWhenTheProviderIsDisposed

@steveharter
Copy link
Member

@alistairjevans the new test is failing; the service is being disposed.

From the issue:

In the MS DI implementation, it is stated in docs that services not created by the service container are not disposed of when the application instance stops.

However from the docs is states that registered singletons are disposed:

The container calls Dispose for the IDisposable types it creates. Services resolved from the container should never be disposed by the developer. If a type or factory is registered as a singleton, the container disposes the singleton automatically.

@steveharter steveharter added the needs-author-action An issue or pull request that requires more info or actions from the author. label Dec 5, 2024
Copy link
Contributor

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

Copy link
Contributor

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

@dotnet-policy-service dotnet-policy-service bot removed this from the 10.0.0 milestone Jan 3, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2025
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-Extensions-DependencyInjection community-contribution Indicates that the PR has been added by a community member needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity
Projects
None yet
2 participants