Redis: Connection Leak if Connect or ConnectAsync called from multiple threads simultaneously

[JonCole]

We have a customer who, under load testing, noticed connections to Redis spiking significantly. After digging into the dump of the process, we could see that many ConnectionMultiplexer objects exist in the heap. We also noticed that there were many threads calling Connect at the same time.

If multiple threads make it past the if (_connection == null) check, then there will be a connection leak for each additional thread that makes it through that check.

[Tratcher]

Relevant code: https://github.com/aspnet/Caching/blob/1.0.0/src/Microsoft.Extensions.Caching.Redis/RedisCache.cs#L146-L162

@JonCole what version of the packages are they using? Do we need to backport this fix to 1.1.x or 1.0.x?

/cc: @muratg

[JonCole]

@Tratcher - looks like they are on 1.1.0.21115.

[muratg]

Putting this in 1.1.1 milestone so that it gets tracked with the rest of 1.1.1 candidates. We should also look into whether we want this in 1.0.4 as well.

[JunTaoLuo]

Fixed in 3522082. Will need to port this to 1.1.1 branch when available for release.

[runxc1]

Would this only happen on startup? I am noticing about once a day or so on a site with 1 million hits a day that Redis will run out of connections but just for 5-10 minutes.

I am using the distributedCache inside a singleton service that uses both the memory and distributed cache for short/medium term caching.

[muratg]

(1.0.4 tracking bug is here: #257)

[Eilon]

This patch bug is approved. Please use the normal code review process w/ a PR and make sure the fix is in the correct branch, then close the bug and mark it as done.

[JunTaoLuo]

Patch merged. Leaving the issue open since version numbers will need to be updated.

[runxc1]

What schedule is this project on for a a new release push to Nuget?

[Eilon]

@runxc1 we're hoping to get these patches out in February.

[MikaelPorttila]

Bump Any release date yet?

[runxc1]

@MikaelPorttila I am following a couple rather large issues in DotNetCore and it seems that all projects are going to ship at the same time as everyone has quoted "February" as when they are going to fix the issues I am watching. I had thought part of the advantage of doing it in the open and disconnecting everything was to be able to release out of bound but it seems that everything is still on a schedule.

[Eilon]

@MikaelPorttila the plan is to have it out later this month. We're just nearly done with all the fixes, and we're working on getting the final builds put together and doing additional testing.

@runxc1 one of the reasons we often prefer to ship things together is that we can ensure that they have maximum compatibility between them. We do sometimes release packages as "one-off" but the cost of testing the additional combinations can be prohibitive.

[JunTaoLuo]

Changes merged. Updating version numbers now.

[runxc1]

Yeah I get that @Eilon and am hoping that waiting on some of these items helps the .Net Standard 2.0 and all it entails to be released sooner. With that said some of these issues like the Kestrel locking make me wish I hadn't used Asp.Net Core yet.

[Eilon]

@runxc1 understood. We're certainly all super excited about .NET Standard 2.0 and everything that it brings. I think as far as Kestrel stability is concerned, though we've of course seen some issues in that space, overall stability has been extremely high, and when we do find stability issues, they tend to get priority treatment for the patch releases (as opposed to "general" bug fixes).