RefreshScope dispose beans in unpredictable order

[brenuart]

This issue was discovered while investigating https://github.com/spring-cloud/spring-cloud-netflix/issues/3174#issuecomment-419349960. To summarise the original tread:

When disposed, RefreshScope starts by clearing its local cache of target beans then proceeds with their disposal. A problem arise when one of these beans makes a call to another refresh-scoped bean in its dispose method - that is the case of Netflix's DiscoveryClient for instance. Since the cache has already been cleared, a new instance of the proxied reference must be created which leads to the BeanCreationNotAllowedException.

RefreshScope extends GenericScope whose shutdown method looks like this: https://github.com/spring-cloud/spring-cloud-commons/blob/master/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java#L130-L134. As you can see it starts by clearing the local cache...

Furthermore, target beans are NOT disposed in an order that matches their dependencies. In fact, beans are disposed in the order they are returned by the StandardScopeCache which uses a ConcurrentHashMap underneath.

Changing the map implementation to preserve order will not be enough I'm afraid. Proxies are created in dependency order, but targets are created - and stored in the cache - when they are first invoked. To be 100% correct, the exact moment a target bean gets into the cache depends on:

• RefreshScope is configured with eager initialization (true by default). In this case, target beans are created on ContextRefreshedEvent in the order given by context.getBeanDefinitionNames() (does it reflect the dependency order?). When eager=false, target beans are created at the very last moment on their first use.
• whether the bean definition is @Lazyor not

A sample test app is available at https://github.com/brenuart/spring-cloud-commons/tree/netflix_gh3174 to illustrate the case. It is currently hosted in my own repo - please tell me if I should commit it elsewhere. You may not find all tests relevant - certainly in the light of the changes you foresee to solve this issue. Hope they will be useful anyway ;-)

[fpdjsns]

@spencergibb Do you have any progress?

[spencergibb]

None.

[fpdjsns]

thank you for announce.

[jebeaudet]

FWIW, I've run into this by using a custom HttpComponentsClientHttpRequestFactory bean along with the EurekaClientHttpRequestFactorySupplier and the Eureka client (we need some custom HttpClient management).

Using the default configuration, the CloudEurekaClient gets instantiated with the @RefreshScope. On shutdown with a normal SIGTERM, the HttpComponentsClientHttpRequestFactory bean gets shutdown before the CloudEurekaClient instance (even though it should be the reverse order of instantiation). This leads to a non graceful shutdown of the Eureka client because the HttpClient is shut down and throwing errors when trying to be used by the shutdown process. Example stack :

com.netflix.discovery.shared.transport.decorator.RedirectingEurekaHttpClient
Request execution error. endpoint=DefaultEndpoint{ serviceUrl='http://redacted_url} exception=Connection pool shut down stacktrace=java.lang.IllegalStateException: Connection pool shut down
    at org.apache.http.util.Asserts.check(Asserts.java:34)
    at org.apache.http.impl.conn.PoolingHttpClientConnectionManager.requestConnection(PoolingHttpClientConnectionManager.java:269)
    at org.apache.http.impl.execchain.MainClientExec.execute(MainClientExec.java:176)
    at org.apache.http.impl.execchain.ProtocolExec.execute(ProtocolExec.java:186)
    at org.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java:185)
    at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:83)
    at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:56)
    at org.springframework.http.client.HttpComponentsClientHttpRequest.executeInternal(HttpComponentsClientHttpRequest.java:87)
    at org.springframework.http.client.AbstractBufferingClientHttpRequest.executeInternal(AbstractBufferingClientHttpRequest.java:48)
    at org.springframework.http.client.AbstractClientHttpRequest.execute(AbstractClientHttpRequest.java:66)
    at org.springframework.web.client.RestTemplate.doExecute(RestTemplate.java:776)
    at org.springframework.web.client.RestTemplate.execute(RestTemplate.java:711)
    at org.springframework.web.client.RestTemplate.exchange(RestTemplate.java:602)
    at org.springframework.cloud.netflix.eureka.http.RestTemplateEurekaHttpClient.sendHeartBeat(RestTemplateEurekaHttpClient.java:99)
    at com.netflix.discovery.shared.transport.decorator.EurekaHttpClientDecorator$3.execute(EurekaHttpClientDecorator.java:92)
    at com.netflix.discovery.shared.transport.decorator.RedirectingEurekaHttpClient.execute(RedirectingEurekaHttpClient.java:91)
    at com.netflix.discovery.shared.transport.decorator.EurekaHttpClientDecorator.sendHeartBeat(EurekaHttpClientDecorator.java:89)
    at com.netflix.discovery.shared.transport.decorator.EurekaHttpClientDecorator$3.execute(EurekaHttpClientDecorator.java:92)
    at com.netflix.discovery.shared.transport.decorator.RetryableEurekaHttpClient.execute(RetryableEurekaHttpClient.java:120)
    at com.netflix.discovery.shared.transport.decorator.EurekaHttpClientDecorator.sendHeartBeat(EurekaHttpClientDecorator.java:89)
    at com.netflix.discovery.shared.transport.decorator.EurekaHttpClientDecorator$3.execute(EurekaHttpClientDecorator.java:92)
    at com.netflix.discovery.shared.transport.decorator.SessionedEurekaHttpClient.execute(SessionedEurekaHttpClient.java:77)
    at com.netflix.discovery.shared.transport.decorator.EurekaHttpClientDecorator.sendHeartBeat(EurekaHttpClientDecorator.java:89)
    at com.netflix.discovery.DiscoveryClient.renew(DiscoveryClient.java:893)
    at com.netflix.discovery.DiscoveryClient$HeartbeatThread.run(DiscoveryClient.java:1457)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:829)

If you don't use the @RefreshScope, you can fix this by disabling it in your configuration spring.cloud.refresh.enabled: false. If you use it though, I guess you could subclass HttpComponentsClientHttpRequestFactory and override the destroy method not to shutdown the HttpClient. This is not 100% graceful but on shutdown it should be harmless.