Skip to content

Conversation

Arkatufus
Copy link
Contributor

Fixes #7426

Changes

  • Use AtomicReference<T> to enforce thread safety

Copy link
Contributor Author

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

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

Self-review

Comment on lines +52 to +58
while(true)
{
var oldConfig = _fallbackConfig.Value;
var newConfig = config.SafeWithFallback(oldConfig);
if (_fallbackConfig.CompareAndSet(oldConfig, newConfig))
break;
}
Copy link
Contributor Author

@Arkatufus Arkatufus Jul 2, 2025

Choose a reason for hiding this comment

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

Fix: Do not assign value to naked field directly, this can cause race condition if multiple Akka extensions running in multiple threads tries to start at the same time.

Scenario:

  • Time 0
    • Extension "A" reads _fallbackConfig into configA
    • Extension "B" reads _fallbackConfig into configB
  • Time 1
    • Extension "A" injects "akka.cluster.tools.singleton" into configA
    • Extension "B" injects "akka.persistence.sql" into configB
  • Time 2
    • Extension "A" writes configA into _fallbackConfig
  • Time 3
    • Extension "B" writes configB into _fallbackConfig
  • Time 4
    • Extension "A" reads "akka.cluster.tools.singleton" from Settings.Config, gets NullReferenceException because it got clobbered by Extension "B"

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@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.

LGTM - that was simple. Let's see how the testing system does.

private readonly Config _userConfig;
//internal static readonly Config AkkaDllConfig = ConfigurationFactory.FromResource<Settings>("Akka.Configuration.Pigeon.conf");
private Config _fallbackConfig;
private readonly AtomicReference<Config> _fallbackConfig;
Copy link
Member

Choose a reason for hiding this comment

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

yeah, this is a much simpler fix than what I was imagining

Comment on lines +52 to +58
while(true)
{
var oldConfig = _fallbackConfig.Value;
var newConfig = config.SafeWithFallback(oldConfig);
if (_fallbackConfig.CompareAndSet(oldConfig, newConfig))
break;
}
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb Aaronontheweb enabled auto-merge (squash) July 2, 2025 18:19
@Aaronontheweb Aaronontheweb disabled auto-merge July 3, 2025 13:46
@Aaronontheweb Aaronontheweb merged commit 857801c into akkadotnet:dev Jul 3, 2025
9 of 11 checks passed
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.

Settings.InjectTopLevelFallback race condition can cause failures
2 participants