orleans
orleans copied to clipboard
Should ActivationData Scope be dispose explicitly in all scenario?
I notice that when an exception is thrown during grain OnActivateAsync /constructor the catalog doesn't dispose of the ActivationData scope.
internal class ActivationData : IGrainActivationContext, IActivationData, IInvokable, IDisposable
{
......
.....
internal void SetupContext(GrainTypeData typeData, IServiceProvider grainServices)
{
this.GrainTypeData = typeData;
this.Items = new Dictionary<object, object>();
this.serviceScope = grainServices.CreateScope();
public interface IServiceScope : IDisposable
The Dispose() method ends the scope lifetime. Once Dispose is called, any scoped services that have been resolved from ServiceProvider will be disposed.
In the default implantation, there is no problem due to the fact the ServiceScope is the only one who references the scope object.
But it depends on the implementation of the IServiceScope I encounter other implantation that must dispose of the scope or it causes a memory leak.
I think we should always dispose of the scope.
Yes, it should. If a scope is created then we must dispose it. Thank you for investigating!
Are you able to open a PR to fix this?
I investigated a memory leak, I decided that the best approach for me is to create other implementation of the service scope eliminating this kind of leaks in the first place.
I don't sure, I have time to create a PR.
The problematic flow:
sequenceDiagram
catalog-->>ActivationData:GetOrCreateActivation
ActivationData-->>ActivationData:constractor
catalog-->>catalog:RegisterMessageTarget(ActivationData )
catalog->>catalog:activations.Add
catalog-->>catalog:SetupActivationInstance
catalog-->>ActivationData:SetupContext() !!
catalog-->>catalog:CreateGrain<throw>
catalog-->>catalog:UnregisterMessageTarget
catalog-->>catalog:activations.Remove
ActivationData-->ActivationData:Missing Dispose like heppand in the FinishDestroyActivations
I missing a lot of context but ,my assumption is that on UnregisterMessageTarget that grain should be dead and can safly dispose of the context .
I think the scope is not disposed also a case of duplicate activation.
@ReubenBond, I read the code, I am trying to understand what can affect failing to create the state due to duplicate activation. As I understand from the code fail to create due to duplicate activation race is affected by a few conditions.
- Time to publish grain directory.
- Placement strategy (local)
- Time to create the Grain.
- OnActivateAsync
- Constructor
What should be the approach to avoid failers to create the grain due to duplicate activation when your grain needs to do long init operation (what is the right stage)?
We've moved this issue to the Backlog. This means that it is not going to be worked on for the coming release. We review items in the backlog at the end of each milestone/release and depending on the team's priority we may reconsider this issue for the following milestone.
ActivationData was rewritten in 7.0 and if there was an issue here, it was likely fixed along with that. I'll close this now but please open a new issue if you still experience this issue