Azure / azure-signalr

Azure SignalR Service SDK for .NET
https://aka.ms/signalr-service
MIT License
412 stars 97 forks source link

AzureSignalRService Stop() and IDisposable #991

Open dazinator opened 3 years ago

dazinator commented 3 years ago

Describe the bug

Looking at these classes: https://github.com/Azure/azure-signalr/search?q=AzureSignalRHostedService&unscoped_q=AzureSignalRHostedService

I can see that there is an IStartupFilter that is responsible for Starting the AzureSignalRHostedService when the middleware pipeline is first built.

I can also see that the AzureSignalRHostedService is registered as a singleton.

However it doesn't appear to implement IDisposable, and there doesn't appear to be any code that gracefully "stops" for example when the container is disposed.

I am seeing exceptions occuring like this:

image

Our architecture is a bit more complicated because we register signalr in child containers, which we can dispose and rebuild at runtime (same with the middleware pipeline / request delegate). However I am concerned that when the container is Disposed, because AzureSignalRHostedService doesn't implement IDisposable, and because it will have been "Started".. should it not "Stop" something when it is disposed? It seems like it something maybe continuing to run and accessing services from a now disposed container.

To Reproduce

I don't think this is easily reproducable unless you happen to have an architecture like ours. However conceptually atleast, register and start the service in a container, and then dispose of the container - it seems like activity may continue using disposed services?

Exceptions (if any)

As per screenshot.

Further technical details

JialinXin commented 3 years ago

Thanks for reporting the issue. I'm looking into this.

dazinator commented 3 years ago

Any updates on this one?

JialinXin commented 3 years ago

Sorry I was working on something else and stopped repro this. I'm guessing this is caused by some loop or object not exit gracefully that triggered by your scenario. I'm wondering there might be other cases as well which needs some further tests for a complete fix. Cause dispose order is not determined.

I'd like to check if this is one time exception or some continues ones annoying you a lot? Anywhere else do you meet similar exceptions?

https://github.com/Azure/azure-signalr/blob/dev/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnection.cs#L323

dazinator commented 3 years ago

I'd like to check if this is one time exception or some continues ones annoying you a lot?

It happens constantly, so much so that our only option was to remove azure signalr and just use locally hosted signalr hubs - which is a workaround we can get away with for a limited time whilst we only have 1 web server node. Once we scale out to more web server nodes we will need to look at scaling signalr and was hoping to use azure signalr for this, but won't be able to with this issue present.

Anywhere else do you meet similar exceptions?

No, have only noticed this one exception as per the stack trace provided.

osmnorhn commented 3 months ago

Do we have any progress on the issue? We are also having the same :)

Basically the issue is, while the application is shutting down, we are getting System.ObjectDisposedException. in the Hub's OnDisconnectedAsync method.