microsoft / service-fabric-aspnetcore

This repo contains ASP.NET Core integration for Service Fabric Reliable Services.
Other
152 stars 49 forks source link

Exception in ConfigureServices() leads to infinite loop #42

Closed helgemahrt closed 6 years ago

helgemahrt commented 6 years ago

In our case, it actually lead to a leak of a disposable Singleton object in the container. Let me explain:

We're setting up our ServiceInstanceListener with the standard code:

        protected override IEnumerable<ServiceInstanceListener> CreateServiceInstanceListeners()
        {
            return new ServiceInstanceListener[]
            {
                new ServiceInstanceListener(serviceContext =>
                    new KestrelCommunicationListener(serviceContext, "ServiceEndpoint", (url, listener) =>
                    {
                        ServiceEventSource.Current.ServiceMessage(serviceContext, $"Starting Kestrel on {url}");

                        return new WebHostBuilder()
                                    .UseKestrel()
                                    .ConfigureServices(
                                        services => services
                                            .AddSingleton<StatelessServiceContext>(serviceContext))
                                    .UseContentRoot(Directory.GetCurrentDirectory())
                                    .UseStartup<Startup>()
                                    .UseServiceFabricIntegration(listener, ServiceFabricIntegrationOptions.None)
                                    .UseUrls(url)
                                    .Build();
                    }))
            };
        }

In our ConfigureServices() function there was an issue down the line, which launched an exception. However, before reaching that exception, we added a singleton to the services. E.g.: services.AddSingleton<IMyService>(new MyServiceImpl());

This class has a timer and is disposable. Here's a simplified example:

    public class MyServiceImpl : IMyService, IDisposable
    {
        private Timer _timer = null;

        public MyServiceImpl()
        {
            _timer = new Timer(1000);
            _timer.Elapsed += _timer_Elapsed;
            _timer.Enabled = true;

            ServiceEventSource.Current.Write("MyServiceImpl instantiated");
        }

        public void Dispose()
        {
            ((IDisposable)_timer).Dispose();
        }

        private void _timer_Elapsed(object sender, ElapsedEventArgs e)
        {
            ServiceEventSource.Current.Write("Timer elapsed");
        }
    }

When the exception is thrown, it takes a moment and then we enter CreateServiceInstanceListeners() again, and call ConfigureServices() again. This creates another instance of MyServiceImpl, but the first one doesn't get disposed. As this is a timer, this will keep the original object alive in memory until the process dies, and it will keep firing away.

In our case, we realized there was an issue after our microservice reached 100% CPU and about 8GB of memory, when there were already more than 1000 events per second fired. I get that this is probably a very arcane scenario, but - unfortunately - we encountered it in a production environment. It's unexpected that 1) the process loops without crashing when the startup of a web app/API fails and 2) the services container doesn't dispose existing objects properly. (I checked, and in subsequent iterations after the first, MyServiceImpl wasn't present in the container yet, so I assume that it has been cleared before starting over)

The only workaround I found for this was to make the timer static and check in the constructor whether it's been instantiated already. If it has, dispose it. This is less than ideal, because it forces this class to be a singleton when I may not want this.

amanbha commented 6 years ago

@helgemahrt This is not related to SF and looks like aspnetcore, I see that you have already opened one in aspnet core.

amanbha commented 6 years ago

Closing this one as its aspnet core issue and tracked in the lined issue in aspnet corer repo.