Ensure that configuration property binding is thread-safe and provide an API for changing the property sources from which it binds

[dsyer]

The CPBPP can be called by Spring after the application has started (e.g. for @ConfigurationProperties beans that are scoped proxies). In that case the cached values of the property sources and rebinder that it created on startup can be stale. It would be great if it "just worked", but if that isn't easy, a public API to reset those things would make it possible to fix the @RefreshScope in Spring Cloud. Currently (since 1.0) we have a hack that sets fields in CPBPP using reflection.

[wilkinsona]

It's difficult to propose a solution without knowing more about the use case. Can you please describe in detail what changes are being made that lead to the property sources and rebinder being stale? We'd also need to know precisely how those changes are being made. For example are they happening on the same thread that honours@RefreshScope or are multiple threads potentially involved here?

[spencergibb]

This is the change (ie the "hack") spring-cloud/spring-cloud-commons@9639787

[dsyer]

All we need is a public reset() to prevent us from having to hack the internal state. I don't think it's relevant how the changes are being made, and we don't really control that anyway (any user can open up the Environment at any time and add new PropertySources).

[wilkinsona]

I don't think it's relevant how the changes are being made

It's completely relevant. Without considering how the change can be made and how that change is consumed, there's no guarantee that it will be thread-safe. Your current hack illustrates the problem perfectly:

• If the post-processor tries to perform configuration property binding on one thread while the hack is running on another there's a window where a NullPointerException will occur
• If the reset is performed on one thread and the binder is recreated on another, there's no guarantee that the changes performed by the reset will be visible as there's no locking and none of the fields involved are volatile

What you're really asking for here is for configuration property binding to be made completely thread-safe while also supporting mutation at any time from any thread. It works now for Boot itself because binding is performed exclusively on the main thread during application context refresh and the binder is created once and it's then reused. Mutation of the property sources works because, typically, they are a MutablePropertySources instance that is backed by CopyOnWriteArrayList.

[dsyer]

We never had anyone complain about race conditions, so it seems likely that they are a corner case. But, of course if it turns out to be important, then we'd be more than happy to see an implementation which takes it into account somehow.

[wilkinsona]

You may be happy dismissing thread-safety in Spring Cloud as a corner case that can be ignored until people complain, but I am not happy doing so in Boot. IMO, we can only consider implementing this if we're certain that it's thread-safe and won't lead to obscure and hard-to-track-down bugs. I haven't looked much below the surface, but my gut feel is that this could be a surprisingly large piece of work, and may not make the cut for 2.0.