Breaking change: Throw when resolving on a disposed service provider

[maryamariyan]

• Resolving Services after ServiceProvider is Disposed should throw with ObjectDisposedException

@davidfowl, for testing I prepared a test similar to the repro code you provided earlier in this comment

Similar to the use case you were illustrating there, this use case (before this PR change) would also result in a deadlock. Here we try to call Dispose while the singleton lock ResolvedServices is taken). As shown in this test now it would throw ObjectDisposedException instead.

[ghost]

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

---

• Resolving Services after ServiceProvider is Disposed should throw with ObjectDisposedException

@davidfowl, for testing I took a test similar to the repro code you provided earlier in this comment

Similar to the use case you were illustrating there where the singleton lock would be used, before this PR change, the test would result in a deadlock since Dispose would use the same "ResolvedServices" singleton lock when resolving singleton services. As shown in this test now it would throw ObjectDisposedException instead.

Author:	maryamariyan
Assignees:	-
Labels:	area-Extensions-DependencyInjection
Milestone:	-

[maryamariyan]

Thanks @davidfowl. I think the PR is ready for another now.

[eerhardt]

What is this testing?

[maryamariyan]

If during resolve of this service, service provider gets disposed on another thread, the test asserts that we dispose of the service as well.

[eerhardt]

But the way the test is written above, this sleep will happen inline and then the object will be returned on line 276.

Once the object is returned, then you are starting a new thread to dispose the service provider.

So I don't think the sleep is doing anything beneficial.

[davidfowl]

Good catch, we need to kick off 2 threads.

[maryamariyan]

If during resolve of this service, service provider gets disposed on another thread, the test asserts that we dispose of the service as well.

Removed this test and instead used tests below to also assert that the service (for both IDisposable and IAsyncDisposable cases) get disposed:

• GetAsyncService_DisposeAsyncOnSameThread_ThrowsAndDoesNotHangAndDisposeAsyncGetsCalled
• GetService_DisposeOnSameThread_ThrowsAndDoesNotHangAndDisposeGetsCalled

The removed case would have been very similar in outcome to the two test cases I mentioned above, so perhaps OK that we don't add it.

[eerhardt]

This looks good to me. Nice work, @maryamariyan.