IdentityServer4.Contrib.RedisStore icon indicating copy to clipboard operation
IdentityServer4.Contrib.RedisStore copied to clipboard

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

Open mknet3 opened this issue 5 years ago • 1 comments

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.

mknet3 avatar Oct 20 '20 10:10 mknet3

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

AliBazzi avatar Oct 20 '20 10:10 AliBazzi