autofac / Autofac.ServiceFabric

Autofac integration for Azure Service Fabric. Provides service factory implementations for Actors, Stateful Services and Stateless Services.
MIT License
26 stars 27 forks source link

How to use Serilog, Autofac and Service Fabric Actors together #38

Closed johnkattenhorn closed 5 years ago

johnkattenhorn commented 6 years ago

I've been asked to come out of coding retirement to look at a production issue :-)

It seems we have a problem with disposing of Serilog objects judging by a memory dump.

In program.cs with register a module with this code

// In some places we don't have the container available so setting Log.Logger static

Log.Logger = ConfigureLogger().CreateLogger();

builder
     .Register(c => Log.Logger)
     .As<ILogger>()
     .SingleInstance();

We then register the Actor like this:

builder.RegisterActor<Ibis2Actor>()
                  .WithParameter(
                      (p, c) => p.ParameterType == typeof(ILogger),
                      (p, c) => c.Resolve<ILogger>().ForContext(new ActorEnricher(c.Resolve<ActorService>(),c.Resolve<ActorId>())));

The Actor Class looks like this:

public class Ibis2Actor : DeviceActor<Ibis2Reader>, IDeviceActor
    {
        public Ibis2Actor(
            ActorService actorService,
            ActorId actorId,
            DeviceContext deviceContext,
            ILogger logger) :
            base(actorService, actorId, deviceContext, logger)
        {
        }
...
}

Where DeviceActor is a abstract class inheriting from Microsoft.ServiceFabric.Actors.Runtime.Actor, maybe significantly neither the abstract class or the concrete class implement IDisposable.

My worry is that when the given ForContext returns an ILogger which does'nt implement IDisposable that this logger does not have cleanup up even after the Actor is deactivated.

Does anyone have a view about this approach or see what might be causing us to hold onto the Serilog objects ?

Should I make the Actor IDisposable or implement some kind of OnRelease in the Actor registration ?

johnkattenhorn commented 6 years ago

So we've confirmed today that the OnDeactivate method is being called on the Actor and that Actor no longer appears in the memorydump but I can't think of a way to remove the child logger from the memory.

@alexmg, @nblumhardt - would simply setting the ILogger to null work ? How about casting the return ILogger to Logger so I can run the Dispose method on it?

Maybe I'm getting hung up in the memory dump; if it's not in Gen2 it should be ok right unless (%) Timespent in GC gets too high.

nblumhardt commented 6 years ago

ForContext returns an ILogger which does'nt implement IDisposable that this logger does not have cleanup up even after the Actor is deactivated.

The loggers returned by ForContext don't implement IDisposable because they don't need any clean-up. If they're hanging around, it'll be because some other retained object is referencing them (or just that GC hasn't run on a high enough generation, yet).

HTH!

tillig commented 5 years ago

Hopefully this has helped; if not, StackOverflow might get you an answer sooner.