Skip to content
This repository has been archived by the owner on Nov 7, 2018. It is now read-only.

IOptionsChangeTokenSource does not work with scoped IConfigureOptions #199

Closed
pakrym opened this issue Jun 14, 2017 · 5 comments
Closed
Assignees
Milestone

Comments

@pakrym
Copy link
Contributor

pakrym commented Jun 14, 2017

Not sure if this is intended behavior but it is confusing.
Consider the following example:

 public class Startup
    {
        // This method gets called by the runtime. Use this method to add services to the container.
        // For more information on how to configure your application, visit https://go.microsoft.com/fwlink/?LinkID=398940
        public void ConfigureServices(IServiceCollection services)
        {
            services.AddScoped<IConfigureOptions<ScopedOptions>, ScopedConfigureOptions>();
        }

        // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
            }

            app.Run(async (context) =>
            {
                var optionsSnapshot = context.RequestServices.GetService<IOptionsSnapshot<ScopedOptions>>();
                await context.Response.WriteAsync(optionsSnapshot.Value.I.ToString());
            });
        }
    }

    public class ScopedConfigureOptions : IConfigureOptions<ScopedOptions>
    {
        private int _i;
        private static int _global;

        public ScopedConfigureOptions()
        {
            _i = _global++;
        }

        public void Configure(ScopedOptions options)
        {
            options.I = _i;
        }
    }

    public class ScopedOptions
    {
        public int I { get; set; }
    }

I would expect OptionsSnapshot being scoped to initialize options instance using new instance of scoped IConfigureOptions every time instead last first one is cached forever (if IOptionsChangeTokenSource is not used)

@HaoK

@pakrym
Copy link
Contributor Author

pakrym commented Jun 15, 2017

Scoped IOptionsChangeTokenSource on the other hand are supported

@HaoK
Copy link
Member

HaoK commented Jun 15, 2017

Yeah this is a bug, the cache is singleton, so even though the snapshot is recreated, the cache'd value is returned.

The snapshot shouldn't be using the IOptionsCache at all, we should consider switching IOptionsMonitor over to the IOptionsCache/IOptionsFactory and having that be the official new way to get live options instances + do cache invaldations

@HaoK
Copy link
Member

HaoK commented Jun 15, 2017

Basically OptionsSnapshot should only be using the factory, and the instance itself is the scoped cache.

@HaoK HaoK added the bug label Jun 15, 2017
@HaoK HaoK added this to the 2.0.0 milestone Jun 15, 2017
@HaoK HaoK self-assigned this Jun 15, 2017
@HaoK
Copy link
Member

HaoK commented Jun 15, 2017

There's a second issue here as a result, snapshot shouldn't really be doing the changeSource registration. I'll file an issue for that now

@kevinchalet
Copy link

Note that you caused a terrible regression when fixing this bug. See aspnet/Security#1282 (comment) for more information.

I'll open a new ticket to ensure it's correctly tracked.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants