Websocket singleton who uses Scoped Service

Question

I have implemented WebSocket middleware who has contains as a field singleton WebSocket dictionary(injected via constructor) and some other scoped parameters injected via constructor.

I would like to know if it's implemented correctly.

public WebSocketManagerMiddleware(RequestDelegate next,
    IWebSocketConnectionDictionary webSocketDictionary,
    IScopedServiceOne scopedOneService, IScopedServiceTwo scopedtwoService)
{
    _next = next;
    _webSocketManager = webSocketDictionary;
    _scopedOneService= scopedOneService;
    _scopedtwoService= scopedtwoService;
}

To this constructor, I am injecting these instances like this:

_app.UseMiddleware<WebSocketManagerMiddleware>(
    app.ApplicationServices.GetWebSocketConnectionDictionary(),
    serviceProvider.CreateScope().ServiceProvider.GetScopedOneService(),
    serviceProvider.CreateScope().ServiceProvider.GetScopedTwoService())

I am afraid that I every time on WebSocket request create new scope from where I am getting scoped services(serviceOne, serviceTwo) and it never disposed until WebSocket connection is closed. Because I am using these services only on the websocket start and after I starting to listen to upcoming messages I never use them (IScopedOneSerice, IScopedTwoService)

public async Task Invoke(HttpContext context, IServiceProvider service)
{
    await _scopedOneService.MethodOne();
    await _scopedTwoService.MethodTwo();
    //startint to listen for messages and if I need to call some repository 
    // method I am using 
    //IServiceProvider, i.e ISomeRepository repo =
    //    service.GetRequiredService<ISomeRepository>(); // this repo scoped as well
}

Is it possible to get any memory leak this way?

UPDATED: What I am trying to achieve: let's make it simple, every time I get a websocket message I need to insert the message to the repository or resolve some other services who communicates with other business logic services.

I am not sure what is the best approach to inject scoped serviecs into websocket middleware who contains singleton websocket dictionary and some other scoped services.

I think you should resolve the scoped services inside the middleware using an injected `IServiceScopeFactory` instead of trying to get them from outside. Replace the scoped services in the constructor with a single `IServiceScopeFactory` and store that. Everytime you have to use one of the scoped services, get them by creating a new scope and resolving them from there. — Joelius, Oct 14 '19 at 12:16
Should I use scoped service with using() so after I use it I could easily dispose? — Andrius, Oct 14 '19 at 12:19
Can you clarify what it is you’re trying to achieve (step by step flow)? — theMayer, Oct 14 '19 at 12:20
1. WebSocket client connects to my middleware. 2. Add a new Websocket to Dictionary. 3. Uses ScopedServiceOne, ScopedServiceTwo to verify if WebSocket connection is correct. 4. If it's verified then I am starting to listen for new messages 5. A new message arrives I resolve some scoped method who calculates data from message and insert into database 6. Send the response to the message — Andrius, Oct 14 '19 at 12:27

score 2 · Accepted Answer · answered Oct 14 '19 at 12:54

2

As said in my comment, you should inject an IServiceScopeFactory instead of the scoped services. You can then use that to resolve the scoped services.

public WebSocketManagerMiddleware(RequestDelegate next,
    IWebSocketConnectionDictionary webSocketDictionary,
    IServiceScopeFactory scopeFactory)
{
    _next = next;
    _webSocketManager = webSocketDictionary;
    _scopeFactory = scopeFactory;
}

You can then create a new scope and resolve the service from there whenever you need to access them.

using (var scope = _scopeFactory.CreateScope())
{
    var scopedOneService = scope.ServiceProvider.GetRequiredService<IScopedServiceOne>();
    var scopedTwoService = scope.ServiceProvider.GetRequiredService<IScopedServiceTwo>();
    // do something with scoped services
}

You should also read this answer which is correct in this context but not in the context of the question Chris' answer was posted for, that's why it's been downvoted :)

answered Oct 14 '19 at 12:54

Joelius

3,839
1
16
36

'Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.' I am getting this error when I'm doing load test when the WebSocket client sends a message every 10ms. Every arrived message invokes the INSERT query to my database. Does it mean I have somewhere a leak? The error comes after 5 minutes the load test was started. From diagnostic tool I see my memory steadily increases until 600mb and CPU usage is 100% – Andrius Oct 14 '19 at 18:40
That does not sound good. I can assure you that simply resolving services using a scope from an `IServiceScopeFactory` will not use that much CPU and memory. I'm assuming you aren't properly disposing your sockets and connections. The leak is not in the code I showed you. You should try to debug the application and step through the method where you use the scope to see what actually happens. I can't tell you much more than to properly dispose all the sockets and connections you open. Also research the classes your using, maybe you should use one instead of many like with `HttpClient`. – Joelius Oct 14 '19 at 19:44
I see there is the same effect without IServiceScopeFactory. Of course, it isn't the reason for all of it. I tried to remove all database repository queries from my WebSocket service logic. Without them, I see it works much better. I am using dapper where if a connection is closed or broken I'm opening a connection.SqlConnection I am using as scoped (registered in DI container). So I do not dispose of it because it's scoped and reuesed. But still, it produces a leak probably – Andrius Oct 14 '19 at 19:55
Everything I do on my local computer now. Maybe for my SQL server every 10ms 3x inserts queries at a time to my database is too much, no free connection left and that's why it's happening? Maybe I am expecting too much from the SQL server with this load? With every 100ms works lots better but all queries process time is definetly longer. – Andrius Oct 14 '19 at 20:06
You should buffer the messages before adding them in bulk. If you get new messages every 10ms, you should consider buffering them in a queue and add them in bulk each second or so. It's still quite a lot of entries and if you keep this system going for a long time you might just run out of primary keys depending on what type you're using for them. Still, I think you should look into buffering them in RAM, adding them in bulk, freeing all ressources and repeat. The actual messages should only be enqueued. The actual DB-part should only dequeue from that queue. No direct connection! – Joelius Oct 14 '19 at 20:20
Yes, that would be an appropriate way to implement it. But here I am only using the load test. In the real-world of my web service this kind amount of calls it's not expected yet. The problem I see that I don't know if it's leaking somewhere or that is excepted behavior because my database DTU is low – Andrius Oct 14 '19 at 20:28
https://stackoverflow.com/questions/58388566/net-core-dapper-sql-all-pool-connection-were-use-getting-timeoutexception-is If someone would like to join a conversation of the problem described above in comments section – Andrius Oct 15 '19 at 06:38

Websocket singleton who uses Scoped Service

1 Answers1