Meter registries are not removed from the global registry when the context is closed

[Nephery]

Summary

When management.metrics.use-global-registry=true (the default), MeterRegistryPostProcessor adds each context's MeterRegistry bean to Micrometer's static Metrics.globalRegistry:

	private void addToGlobalRegistryIfNecessary(MeterRegistry meterRegistry) {
		if (this.properties.getObject().isUseGlobalRegistry() && !isGlobalRegistry(meterRegistry)) {
			Metrics.addRegistry(meterRegistry);
		}
	}

https://github.com/spring-projects/spring-boot/blob/v4.0.6/module/spring-boot-micrometer-metrics/src/main/java/org/springframework/boot/micrometer/metrics/autoconfigure/MeterRegistryPostProcessor.java#L123-L127

On context close, MetricsAutoConfiguration$MeterRegistryCloser calls meterRegistry.close() but never calls Metrics.removeRegistry(meterRegistry) (or equivalently Metrics.globalRegistry.remove(meterRegistry)):

		@Override
		public void onApplicationEvent(ContextClosedEvent event) {
			if (this.context.equals(event.getApplicationContext())) {
				for (MeterRegistry meterRegistry : this.meterRegistries) {
					if (!meterRegistry.isClosed()) {
						meterRegistry.close();
					}
				}
			}
		}

https://github.com/spring-projects/spring-boot/blob/v4.0.6/module/spring-boot-micrometer-metrics/src/main/java/org/springframework/boot/micrometer/metrics/autoconfigure/MetricsAutoConfiguration.java#L115-L117

This leads to memory leaks if closed MeterRegistrys are not cleaned up from the global registry. Because if the static field, Metrics.globalRegistry, still holds a reference to a closed MeterRegistry, the JVM garbage collector would not be able to cleanup the MeterRegistry instance and its held objects.

Suggested fix

In MeterRegistryCloser, call Metrics.removeRegistry(meterRegistry) or Metrics.globalRegistry.remove(meterRegistry) before closing the meterRegistry:

		public void onApplicationEvent(ContextClosedEvent event) {
			if (this.context.equals(event.getApplicationContext())) {
				for (MeterRegistry meterRegistry : this.meterRegistries) {
					Metrics.globalRegistry.remove(meterRegistry);
					if (!meterRegistry.isClosed()) {
						meterRegistry.close();
					}
				}
			}
		}

Note: Unlike when adding them to the globalRegistry, which was conditional on management.metrics.use-global-registry=true, its probably fine to just unconditionally remove the MeterRegistry here since its being closed anyway?

Reproduction

To see the bug:

1. Pull down my reproduction project: https://github.com/Nephery/spring-boot-micrometer-global-registry-leak
2. Run the test class: MeterRegistryGlobalRegistryLeakTest.java
3. The test will fail because Metrics.globalRegistry is holding on to a monotonously ever-increasing list of closed MeterRegistry instances.
4. This test class will also create 2 heap dump files for the first and last iterations, leak-iter-0.hprof and leak-iter-19.hprof. Notice that by leak-iter-19.hprof, the number of AnnotationConfigApplicationContext in the heap increases from 1 to 20.

Notice the statically from globalRegistry of io.micrometer.core.instrument.Metrics lines in the images below.

AnnotationConfigApplicationContext instances in leak-iter-0.hprof:

AnnotationConfigApplicationContext instances in leak-iter-19.hprof:

To try out the suggested fix:

1. Pull down my branch cleanup-micrometer-globalregistry
   • Note: To minimize deviation from an actual release, this is just a branch off the 4.0.6 tag with Nephery@3f7277a applied for the suggested fix.
2. Run gradle module:spring-boot-micrometer-metrics:publishToMavenLocal
3. Uncomment this line: https://github.com/Nephery/spring-boot-micrometer-global-registry-leak/blob/main/pom.xml#L36
4. Run the test again and notice that it passes now since global registry is now cleaned up.
5. Inspecting the leak-iter-19.hprof heap dump, notice that there's now always ever only one AnnotationConfigApplicationContext in the heap (the last one presumably still in scope due to test reference to it).

AnnotationConfigApplicationContext instances in leak-iter-19.hprof:

There is now NO statically from globalRegistry of io.micrometer.core.instrument.Metrics lines in the heap dump image.

[kwondh5217]

I'd like to work on this. can you assign this to me?

[philwebb]

Thanks @kwondh5217, I've assigned the issue to you.

[wilkinsona]

Closing in favor of #50235. Thanks for the PR, @kwondh5217.