Skip to content

Commit f6c8a65

Browse files
garyrussellartembilan
authored andcommitted
GH-2733: Fix Meter Memory Leak
Fixes #2733 `AbstractMessageChannel` exception meters caused a memory leak because they are stored in a `Set` for destruction purposes but did not implement `hashCode` and `equals`. Add `hashCode()` and `equals()` to all meter facades, delegating to the underlying Micrometer objects.
1 parent 6391e4b commit f6c8a65

File tree

4 files changed

+65
-8
lines changed

4 files changed

+65
-8
lines changed

spring-integration-core/src/main/java/org/springframework/integration/channel/AbstractMessageChannel.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -555,8 +555,9 @@ private Message<?> convertPayloadIfNecessary(Message<?> message) {
555555
protected abstract boolean doSend(Message<?> message, long timeout);
556556

557557
@Override
558-
public void destroy() throws Exception {
558+
public void destroy() throws Exception { // NOSONAR TODO: remove throws in 5.2
559559
this.meters.forEach(MeterFacade::remove);
560+
this.meters.clear();
560561
}
561562

562563
/**

spring-integration-core/src/main/java/org/springframework/integration/channel/AbstractPollableChannel.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -233,7 +233,7 @@ public boolean hasExecutorInterceptors() {
233233
protected abstract Message<?> doReceive(long timeout);
234234

235235
@Override
236-
public void destroy() throws Exception {
236+
public void destroy() throws Exception { // NOSONAR TODO: remove throws in 5.2
237237
super.destroy();
238238
if (this.receiveCounter != null) {
239239
this.receiveCounter.remove();

spring-integration-core/src/main/java/org/springframework/integration/support/management/micrometer/MicrometerMetricsCaptor.java

+40-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2018 the original author or authors.
2+
* Copyright 2018-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -195,6 +195,19 @@ public void record(long time, TimeUnit unit) {
195195
this.timer.record(time, unit);
196196
}
197197

198+
@Override
199+
public int hashCode() {
200+
return this.timer.hashCode();
201+
}
202+
203+
@Override
204+
public boolean equals(Object obj) {
205+
if (!MicroTimer.class.equals(obj.getClass())) {
206+
return false;
207+
}
208+
return this.timer.equals(((MicroTimer) obj).timer);
209+
}
210+
198211
}
199212

200213
protected static class MicroCounterBuilder implements CounterBuilder {
@@ -246,6 +259,19 @@ public void increment() {
246259
this.counter.increment();
247260
}
248261

262+
@Override
263+
public int hashCode() {
264+
return this.counter.hashCode();
265+
}
266+
267+
@Override
268+
public boolean equals(Object obj) {
269+
if (!MicroCounter.class.equals(obj.getClass())) {
270+
return false;
271+
}
272+
return this.counter.equals(((MicroCounter) obj).counter);
273+
}
274+
249275
}
250276

251277
protected static class MicroGaugeBuilder implements GaugeBuilder {
@@ -292,6 +318,19 @@ protected Gauge getMeter() {
292318
return this.gauge;
293319
}
294320

321+
@Override
322+
public int hashCode() {
323+
return this.gauge.hashCode();
324+
}
325+
326+
@Override
327+
public boolean equals(Object obj) {
328+
if (!MicroGauge.class.equals(obj.getClass())) {
329+
return false;
330+
}
331+
return this.gauge.equals(((MicroGauge) obj).gauge);
332+
}
333+
295334
}
296335

297336
}

spring-integration-core/src/test/java/org/springframework/integration/support/management/micrometer/MicrometerMetricsTests.java

+21-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2018 the original author or authors.
2+
* Copyright 2018-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,8 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.assertj.core.api.Assertions.fail;
2121

22+
import java.util.Set;
23+
2224
import org.junit.Test;
2325
import org.junit.runner.RunWith;
2426

@@ -40,11 +42,13 @@
4042
import org.springframework.integration.core.MessageSource;
4143
import org.springframework.integration.endpoint.AbstractMessageSource;
4244
import org.springframework.integration.handler.AbstractReplyProducingMessageHandler;
45+
import org.springframework.integration.test.util.TestUtils;
4346
import org.springframework.messaging.Message;
4447
import org.springframework.messaging.MessageHandler;
4548
import org.springframework.messaging.MessagingException;
4649
import org.springframework.messaging.PollableChannel;
4750
import org.springframework.messaging.support.GenericMessage;
51+
import org.springframework.test.annotation.DirtiesContext;
4852
import org.springframework.test.context.junit4.SpringRunner;
4953

5054
import io.micrometer.core.instrument.MeterRegistry;
@@ -58,6 +62,7 @@
5862
*
5963
*/
6064
@RunWith(SpringRunner.class)
65+
@DirtiesContext
6166
public class MicrometerMetricsTests {
6267

6368
@Autowired
@@ -84,17 +89,26 @@ public class MicrometerMetricsTests {
8489
@Autowired
8590
private NullChannel nullChannel;
8691

92+
@SuppressWarnings("unchecked")
8793
@Test
8894
public void testSend() throws Exception {
8995
GenericMessage<String> message = new GenericMessage<>("foo");
9096
this.channel.send(message);
9197
try {
92-
this.channel.send(this.source.receive());
98+
this.channel.send(this.source.receive()); // "bar"
99+
fail("Expected exception");
100+
}
101+
catch (MessagingException e) {
102+
assertThat(e.getCause().getMessage()).isEqualTo("testErrorCount");
103+
}
104+
try {
105+
this.channel.send(new GenericMessage<>("bar"));
93106
fail("Expected exception");
94107
}
95108
catch (MessagingException e) {
96109
assertThat(e.getCause().getMessage()).isEqualTo("testErrorCount");
97110
}
111+
assertThat(TestUtils.getPropertyValue(this.channel, "meters", Set.class)).hasSize(2);
98112
this.channel2.send(message);
99113
this.queue.send(message);
100114
this.queue.send(message);
@@ -141,12 +155,12 @@ public void testSend() throws Exception {
141155
assertThat(registry.get("spring.integration.send")
142156
.tag("name", "channel")
143157
.tag("result", "failure")
144-
.timer().count()).isEqualTo(1);
158+
.timer().count()).isEqualTo(2);
145159

146160
assertThat(registry.get("spring.integration.send")
147161
.tag("name", "eipMethod.handler")
148162
.tag("result", "failure")
149-
.timer().count()).isEqualTo(1);
163+
.timer().count()).isEqualTo(2);
150164

151165
assertThat(registry.get("spring.integration.receive")
152166
.tag("name", "queue")
@@ -202,6 +216,9 @@ public void testSend() throws Exception {
202216
catch (MeterNotFoundException e) {
203217
assertThat(e).hasMessageContaining("No meter with name 'spring.integration.receive' was found");
204218
}
219+
this.channel.destroy();
220+
assertThat(TestUtils.getPropertyValue(this.channel, "meters", Set.class)).hasSize(0);
221+
205222
}
206223

207224
@Configuration

0 commit comments

Comments
 (0)