Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Address the feedback
  • Loading branch information
tarekgh committed Aug 19, 2022
commit efacec89a244da91e49e2f7ddc1766e0cbec8234
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,11 @@ static async ValueTask Await(int i, ValueTask vt, List<object> toDispose)

}

if (IsRootScope)
if (IsRootScope && !RootProvider.IsDisposed())
{
// If this ServiceProviderEngineScope instance is a root scope, disposing this instance will need to dispose the RootProvider too.
// Otherwise the RootProvider will never get disposed and will leak.
// Note, if the RootProvider get disposed first, it will automatically dispose all attached ServiceProviderEngineScope objects.
RootProvider.Dispose();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why cant this just noop internally?

Copy link
Member Author

@tarekgh tarekgh Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is simpler. no?

More code, more bugs :-) imagine in the future we add anything to the ServicePrivder dispose method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why cant this just noop internally?

By "just noop internally", you mean adding the if check in ServiceProvider Dispose?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ internal ServiceProvider(ICollection<ServiceDescriptor> serviceDescriptors, Serv
/// <returns>The service that was produced.</returns>
public object? GetService(Type serviceType) => GetService(serviceType, Root);

internal bool IsDisposed() => _disposed;

/// <inheritdoc />
public void Dispose()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public void DoubleDisposeWorks()
}

[Fact]
public void RootDisposeTest()
public void RootEngineScopeDisposeTest()
{
var services = new ServiceCollection();
ServiceProvider sp = services.BuildServiceProvider();
Expand Down