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.

Sign up for GitHub

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

Jump to bottom

Fixed issues with duplicate IServiceProvider registration #32

Merged
merged 4 commits into from
Apr 9, 2022
Merged

Fixed issues with duplicate IServiceProvider registration #32

merged 4 commits into from
Apr 9, 2022

Conversation

Aaronontheweb
Copy link
Member

@Aaronontheweb Aaronontheweb commented Apr 9, 2022 •
edited
Loading

This fixes a bug I noticed while adding Akka.Hosting support to Phobos

https://twitter.com/Aaronontheweb/status/1512497162948595717 - this turned out to be caused by Akka.Hosting calling ServiceCollection.BuildProvider() in multiple instances - which causes the service registration delegates to be called multiple times, thus the version of a dependency your ActorSystem is running with can be different than the one ASP.NET might resolve.

Changes

We've re-arranged the internals around Microsoft.Extensions.DependencyInjection inside Akka.Hosting to never actually instantiate the IServiceProvider ourselves - we let the runtime handle that so we end up with a consistent, single instance across the entire IHost.

We've also changed it such that the AkkaHostedService will terminate the entire application in the event of a failure at startup - whereas before it would simply error out in the background without saying anything.

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

@Aaronontheweb Aaronontheweb added the bug Something isn't working label Apr 9, 2022
Aaronontheweb
Aaronontheweb commented Apr 9, 2022
View reviewed changes
Copy link
Member Author

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

Reviewed my changes - found a few things I should clean up

src/Akka.Hosting/AkkaConfigurationBuilder.cs Show resolved Hide resolved
src/Akka.Hosting/AkkaConfigurationBuilder.cs Outdated

private static Func<IServiceProvider, ActorSystem> ActorSystemFactory()
Copy link
Member Author

Choose a reason for hiding this comment

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

The ActorSystem construction now uses the DI container to resolve the AkkaConfigurationBuilder in order to keep context boundaries clean. As a result most of the private members of the AkkaConfigurationBuilder are now internal so they can be accessed from this delegate.

src/Akka.Hosting/AkkaHostedService.cs Outdated Show resolved Hide resolved
src/Akka.Hosting/AkkaHostingExtensions.cs
@@ -50,14 +47,21 @@ public static IServiceCollection AddAkka(this IServiceCollection services, strin
/// Starts a background <see cref="IHostedService"/> that runs the <see cref="ActorSystem"/>
/// and manages its lifecycle in accordance with Akka.NET best practices.
/// </remarks>
public static IServiceCollection AddAkka(this IServiceCollection services, string actorSystemName, Action<AkkaConfigurationBuilder, ServiceProvider> builder)
public static IServiceCollection AddAkka(this IServiceCollection services, string actorSystemName, Action<AkkaConfigurationBuilder, IServiceProvider> builder)
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed the signature here - this is binary compatible.

src/Akka.Hosting/AkkaHostingExtensions.cs
{
var b = new AkkaConfigurationBuilder(services, actorSystemName);
var sp = services.BuildServiceProvider();
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the key fix - can't do this anymore.

src/Akka.Hosting/AkkaHostingExtensions.cs
services.AddSingleton<AkkaConfigurationBuilder>(sp =>
{

builder(b, sp);
Copy link
Member Author

Choose a reason for hiding this comment

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

Run the builder inside DI resolution scope.

src/Akka.Hosting/AkkaHostingExtensions.cs

// registers the hosted services and begins execution
b.Build();
b.Bind();
Copy link
Member Author

Choose a reason for hiding this comment

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

Run the DI bindings exposed by Akka.NET outside the DI resolution scope so they're available as soon as the IHost starts.

@Aaronontheweb Aaronontheweb enabled auto-merge (squash) April 9, 2022 14:39
@Aaronontheweb Aaronontheweb merged commit f1965b5 into akkadotnet:dev Apr 9, 2022
@Aaronontheweb Aaronontheweb mentioned this pull request Apr 9, 2022
Merged
Aaronontheweb added a commit that referenced this pull request May 24, 2022
@Aaronontheweb @eaba
@dependabot @CumpsD @SeanFarrow
v0.3.0 Release (#54)
a41b4f8 
* Fixed tags (#14)

* Ported over changes in the Petabridge.Library template (#15)

* Upgrade to Akka.NET v1.4.35 (#16)

Akka.Hosting was affected by akkadotnet/akka.net#5728

* Update RELEASE_NOTES.md (#17)

* Bump Akka.Persistence.SqlServer from 1.4.32 to 1.4.35 (#18)

Bumps [Akka.Persistence.SqlServer](https://github.com/akkadotnet/Akka.Persistence.SqlServer) from 1.4.32 to 1.4.35.
- [Release notes](https://github.com/akkadotnet/Akka.Persistence.SqlServer/releases)
- [Changelog](https://github.com/akkadotnet/Akka.Persistence.SqlServer/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/Akka.Persistence.SqlServer@1.4.32...1.4.35)

---
updated-dependencies:
- dependency-name: Akka.Persistence.SqlServer
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* removed `ClusterShardingSettings` from sharding extensions (#20)

* Bump docfx.console from 2.59.0 to 2.59.1 (#19)

Bumps [docfx.console](https://github.com/dotnet/docfx) from 2.59.0 to 2.59.1.
- [Release notes](https://github.com/dotnet/docfx/releases)
- [Changelog](https://github.com/dotnet/docfx/blob/dev/RELEASENOTE.md)
- [Commits](dotnet/docfx@v2.59.0...v2.59.1)

---
updated-dependencies:
- dependency-name: docfx.console
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* added `ShardRegionProxy` support to cluster hosting

* added `DistributedPubSub` support

* added v0.1.3 release notes

* Added `WithCustomSerializer` builder method (#22)

* added support for registering custom serializers to `ActorSystem`

* added v0.1.4 Release Notes

* Bump FluentAssertions from 6.5.1 to 6.6.0 (#24)

Bumps [FluentAssertions](https://github.com/fluentassertions/fluentassertions) from 6.5.1 to 6.6.0.
- [Release notes](https://github.com/fluentassertions/fluentassertions/releases)
- [Changelog](https://github.com/fluentassertions/fluentassertions/blob/develop/AcceptApiChanges.ps1)
- [Commits](fluentassertions/fluentassertions@6.5.1...6.6.0)

---
updated-dependencies:
- dependency-name: FluentAssertions
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump AkkaVersion from 1.4.35 to 1.4.36 (#27)

Bumps `AkkaVersion` from 1.4.35 to 1.4.36.

Updates `Akka.DependencyInjection` from 1.4.35 to 1.4.36
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.35...1.4.36)

Updates `Akka.Remote` from 1.4.35 to 1.4.36
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.35...1.4.36)

Updates `Akka.Cluster.Sharding` from 1.4.35 to 1.4.36
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.35...1.4.36)

Updates `Akka.Persistence.Query.Sql` from 1.4.35 to 1.4.36
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.35...1.4.36)

---
updated-dependencies:
- dependency-name: Akka.DependencyInjection
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Remote
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Cluster.Sharding
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Persistence.Query.Sql
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* cleanup WithSqlServerPersistence style (#26)

* add Akka.Persistence.PostgreSql.Hosting support (#28)

* Added v0.1.5 Release Notes (#29)

## [0.1.5] / 06 April 2022
- Added `Akka.Persistence.PostgreSql.Hosting` NuGet package to support Postgres users.

* added DI sanity check specs (#31)

* Fixed issues with duplicate `IServiceProvider` registration (#32)

* commits

* fixed DI registration

* fixed all remaining DI and instantiation issues

* clean up

* added Akka.Hosting v0.2.0 release notes (#34)

* added v0.2.1 release notes (#35)

* Resolve Akka.Remote host binding issue (#37)

* adding Akka.Remote.Hosting specs

* resolved issue

* Added 0.2.2 release notes (#38)

## [0.2.2] / 10 April 2022
- [Bugfix: Akka.Remote.Hosting doesn't support `public-hostname` correctly](#36)

* Bump docfx.console from 2.59.1 to 2.59.2 (#39)

Bumps [docfx.console](https://github.com/dotnet/docfx) from 2.59.1 to 2.59.2.
- [Release notes](https://github.com/dotnet/docfx/releases)
- [Changelog](https://github.com/dotnet/docfx/blob/dev/RELEASENOTE.md)
- [Commits](dotnet/docfx@v2.59.1...v2.59.2)

---
updated-dependencies:
- dependency-name: docfx.console
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Added link to explainer video

* Update README.md

* Bump AkkaVersion from 1.4.36 to 1.4.37 (#41)

Bumps `AkkaVersion` from 1.4.36 to 1.4.37.

Updates `Akka.DependencyInjection` from 1.4.36 to 1.4.37
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.36...1.4.37)

Updates `Akka.Remote` from 1.4.36 to 1.4.37
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.36...1.4.37)

Updates `Akka.Cluster.Sharding` from 1.4.36 to 1.4.37
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.36...1.4.37)

Updates `Akka.Persistence.Query.Sql` from 1.4.36 to 1.4.37
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/dev/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.36...1.4.37)

---
updated-dependencies:
- dependency-name: Akka.DependencyInjection
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Remote
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Cluster.Sharding
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Persistence.Query.Sql
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add interfaces for the ActorRegistry to allow mocking in tests (#42)

* Add interfaces to allow the ActorRegistry to be used in unit tests.

* Add interfaces to allow the ActorRegistry to be used in unit tests.

* Make sure everything compiles again after upstream changes.

* Ensure the IActorRegistry and IReadOnlyActorRegistry are registered with DI.

Co-authored-by: Aaron Stannard <aaron@petabridge.com>

* Bump FluentAssertions from 6.6.0 to 6.7.0 (#47)

Bumps [FluentAssertions](https://github.com/fluentassertions/fluentassertions) from 6.6.0 to 6.7.0.
- [Release notes](https://github.com/fluentassertions/fluentassertions/releases)
- [Changelog](https://github.com/fluentassertions/fluentassertions/blob/develop/AcceptApiChanges.ps1)
- [Commits](fluentassertions/fluentassertions@6.6.0...6.7.0)

---
updated-dependencies:
- dependency-name: FluentAssertions
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump AkkaVersion from 1.4.37 to 1.4.38 (#45)

Bumps `AkkaVersion` from 1.4.37 to 1.4.38.

Updates `Akka.DependencyInjection` from 1.4.37 to 1.4.38
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/1.4.38/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.37...1.4.38)

Updates `Akka.Remote` from 1.4.37 to 1.4.38
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/1.4.38/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.37...1.4.38)

Updates `Akka.Cluster.Sharding` from 1.4.37 to 1.4.38
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/1.4.38/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.37...1.4.38)

Updates `Akka.Persistence.Query.Sql` from 1.4.37 to 1.4.38
- [Release notes](https://github.com/akkadotnet/akka.net/releases)
- [Changelog](https://github.com/akkadotnet/akka.net/blob/1.4.38/RELEASE_NOTES.md)
- [Commits](akkadotnet/akka.net@1.4.37...1.4.38)

---
updated-dependencies:
- dependency-name: Akka.DependencyInjection
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Remote
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Cluster.Sharding
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Akka.Persistence.Query.Sql
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump Microsoft.NET.Test.Sdk from 17.1.0 to 17.2.0 (#46)

Bumps [Microsoft.NET.Test.Sdk](https://github.com/microsoft/vstest) from 17.1.0 to 17.2.0.
- [Release notes](https://github.com/microsoft/vstest/releases)
- [Commits](microsoft/vstest@v17.1.0...v17.2.0)

---
updated-dependencies:
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* close #48 - fixed source of NRE upon shutdown (#49)

* close #48 - fixed source of NRE upon shutdown

* added NRE check in `StopAsync`

* added throw-able `ActorRegistry.Register` method (#50)

* added throw-able `ActorRegistry.Register` method

* Addressed comments

* added null `IActorRef` checks

* adding `ClusterSingleton` hosting methods (#51)

* adding `ClusterSingleton` hosting methods

* fixed compilation errors

* added test output logger

* fixed package metadata issue

* fixed bug with singleton proxy startup

* made tests more configurable

* working on fixing specs

* fixed ClusterSingletonProxy configuration

* fixed bug with singleton and proxy method

* Update AkkaClusterHostingExtensions.cs

* fixed NuGet icon and packaging metadata (#52)

* added 0.3.0 release notes (#53)

Co-authored-by: Ebere Abanonu <eaba@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: David Cumps <david@cumps.be>
Co-authored-by: Sean Farrow <SeanFarrow@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@Aaronontheweb