-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Open
Labels
DXDeveloper experience issues - papercuts, footguns, and other non-bug problems.Developer experience issues - papercuts, footguns, and other non-bug problems.akka-actorpotential bug
Milestone
Description
Version Information
Version of Akka.NET? v1.5.52
Which Akka.NET Modules? Akka
Describe the bug
Users should not be able to modify IWithTimers
akka.net/src/core/Akka/Actor/Scheduler/IWithTimers.cs
Lines 13 to 20 in 1f7ffa7
| public interface IWithTimers | |
| { | |
| /// <summary> | |
| /// Gets or sets the TimerScheduler. This will be automatically populated by the framework in base constructor. | |
| /// Implement this as an auto property. | |
| /// </summary> | |
| ITimerScheduler Timers { get; set; } | |
| } |
The reason why is we automatically set this in the ActorBase constructor
akka.net/src/core/Akka/Actor/ActorBase.cs
Lines 122 to 131 in 1f7ffa7
| protected ActorBase() | |
| { | |
| if (ActorCell.Current == null) | |
| throw new ActorInitializationException("Do not create actors using 'new', always create them using an ActorContext/System"); | |
| if (this is IWithTimers withTimers) | |
| withTimers.Timers = new Scheduler.TimerScheduler(Context); | |
| Context.Become(Receive); | |
| } |
Problems are:
- If user sets this, the
ActorBaseclass is going to reset it, which will cause any timers scheduled in the CTOR to get lost possibly - VS / any other IDE is going to complain about nullability / et al having this unset
Timersproperty.
Solution
- Force users to set the timer scheduler themselves
- Make it a built-in property of
ActorBaseand not require users to decorate. Have timers built-in to all actors automatically. There will be a small perf hit from this on the message receiving pipeline but it's not the end of the world. - Add a Roslyn Analyzer warning telling users not to set it, which is dumb because it means the users are going to get complained at either way.
- In newer versions of .NET, maybe this can be solved with default interface implementations.
Additional Context
Stashing also has this problem!
Metadata
Metadata
Assignees
Labels
DXDeveloper experience issues - papercuts, footguns, and other non-bug problems.Developer experience issues - papercuts, footguns, and other non-bug problems.akka-actorpotential bug