AliBazzi / IdentityServer4.Contrib.RedisStore

A persistence layer using Redis DB for operational data and for caching capability for Identity Server 4
https://www.nuget.org/packages/IdentityServer4.Contrib.RedisStore
MIT License
137 stars 48 forks source link

Lazy initialization of ConnectionMultiplexer is not working if throw exception first time #48

Open mknet3 opened 3 years ago

mknet3 commented 3 years ago

Hi,

When ConnectionMultiplexer is initialized and fails to connect to Redis will fail next times because the exception is cached and thrown again on subsequent calls to the Value property. This is because the property LazyThreadSafetyMode in Lazy object is not set (and then is None).

To fix this property can be set to PublicationOnly:

From:

       internal RedisOptions()
        {
            this.multiplexer = GetConnectionMultiplexer();
        }

        private Lazy<IConnectionMultiplexer> GetConnectionMultiplexer()
        {
            return new Lazy<IConnectionMultiplexer>(
                () =>
                {
                    // if the user provided a multiplexer, we should use it
                    if (this.providedMultiplexer != null)
                    {
                        return this.providedMultiplexer;
                    }

                    // otherwise we must make our own connection
                    return string.IsNullOrEmpty(this.RedisConnectionString)
                        ? ConnectionMultiplexer.Connect(this.ConfigurationOptions)
                        : ConnectionMultiplexer.Connect(this.RedisConnectionString);
                });
        }

        private IConnectionMultiplexer providedMultiplexer = null;
        private Lazy<IConnectionMultiplexer> multiplexer = null;

        internal IConnectionMultiplexer Multiplexer => this.multiplexer.Value;

To:

       internal RedisOptions()
        {
            this.multiplexer = GetConnectionMultiplexer();
        }

        private Lazy<IConnectionMultiplexer> GetConnectionMultiplexer()
        {
            return new Lazy<IConnectionMultiplexer>(
                () =>
                {
                    // if the user provided a multiplexer, we should use it
                    if (this.providedMultiplexer != null)
                    {
                        return this.providedMultiplexer;
                    }

                    // otherwise we must make our own connection
                    return string.IsNullOrEmpty(this.RedisConnectionString)
                        ? ConnectionMultiplexer.Connect(this.ConfigurationOptions)
                        : ConnectionMultiplexer.Connect(this.RedisConnectionString);
                },
                LazyThreadSafetyMode.PublicationOnly);
        }

        private IConnectionMultiplexer providedMultiplexer = null;
        private Lazy<IConnectionMultiplexer> multiplexer = null;

        internal IConnectionMultiplexer Multiplexer => this.multiplexer.Value;

This can be too optional to avoid problems in multithreading environments.

AliBazzi commented 3 years ago

Thanks for pointing this out, can you open a PR ? otherwise I will work on it but on my own time later.