Skip to content

Commit 4856dcb

Browse files
committed
fix PcpMmvWriter strings leak
`PcpMmvWriter` leaks strings through its `PcpStringStore` when the writer is reset. The fix is to clear the store in the reset method. However, the `metricInfoStore` and `instanceDomainStore` rely on putting values in the string store when MMV2 is used. As such, these stores must also be cleared so that the strings can be recreated.
1 parent d9c6522 commit 4856dcb

File tree

4 files changed

+53
-2
lines changed

4 files changed

+53
-2
lines changed

dxm/pom.xml

+5-2
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,10 @@
7676
<artifactId>hamcrest-all</artifactId>
7777
<scope>test</scope>
7878
</dependency>
79-
80-
79+
<dependency>
80+
<groupId>org.slf4j</groupId>
81+
<artifactId>slf4j-simple</artifactId>
82+
<scope>test</scope>
83+
</dependency>
8184
</dependencies>
8285
</project>

dxm/src/main/java/io/pcp/parfait/dxm/PcpMmvWriter.java

+8
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,9 @@ public void reset() {
339339
started = false;
340340
metricData.clear();
341341
perMetricByteBuffers.clear();
342+
instanceDomainStore.clear();
343+
metricInfoStore.clear();
344+
stringStore.clear();
342345
}
343346

344347
@Override
@@ -764,6 +767,11 @@ synchronized Collection<T> all() {
764767
synchronized int size() {
765768
return byName.size();
766769
}
770+
771+
synchronized void clear() {
772+
byId.clear();
773+
byName.clear();
774+
}
767775
}
768776

769777
@Override

dxm/src/main/java/io/pcp/parfait/dxm/PcpString.java

+3
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ Collection<PcpString> getStrings() {
8181
return stringInfo;
8282
}
8383

84+
void clear() {
85+
stringInfo.clear();
86+
}
8487
}
8588

8689
}

dxm/src/test/java/io/pcp/parfait/dxm/PcpMmvWriterIntegrationTest.java

+37
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,14 @@
2121
import org.junit.BeforeClass;
2222
import org.junit.Test;
2323

24+
import java.lang.reflect.Field;
25+
2426
import static io.pcp.parfait.dxm.IdentifierSourceSet.DEFAULT_SET;
2527
import static io.pcp.parfait.dxm.MmvVersion.MMV_VERSION1;
2628
import static io.pcp.parfait.dxm.MmvVersion.MMV_VERSION2;
2729
import static org.hamcrest.MatcherAssert.assertThat;
2830
import static org.hamcrest.core.Is.is;
31+
import static org.junit.Assert.assertEquals;
2932
import static tech.units.indriya.AbstractUnit.ONE;
3033

3134
public class PcpMmvWriterIntegrationTest {
@@ -79,11 +82,45 @@ public void mmvVersion2ShouldSupportInstanceNamesOver63Characters() throws Excep
7982
assertMetric("mmv.v2.integer[instance_name_over_63_characters_instance_name_over_63_characters_instance]", is("11.000"));
8083
}
8184

85+
@Test
86+
public void resetShouldClearStrings() throws Exception {
87+
pcpMmvWriterV1.reset();
88+
assertStringsCount(pcpMmvWriterV1, 0);
89+
pcpMmvWriterV1.addMetric(MetricName.parse("v1.string"), Semantics.DISCRETE, null, "test1");
90+
pcpMmvWriterV1.start();
91+
92+
pcpMmvWriterV2.reset();
93+
assertStringsCount(pcpMmvWriterV2, 0);
94+
pcpMmvWriterV2.addMetric(MetricName.parse("v2.string"), Semantics.DISCRETE, null, "test2");
95+
pcpMmvWriterV2.start();
96+
97+
waitForReload();
98+
99+
assertMetric("mmv.v1.string", is("\"test1\""));
100+
assertMetric("mmv.v2.string", is("\"test2\""));
101+
102+
assertStringsCount(pcpMmvWriterV1, 1);
103+
assertStringsCount(pcpMmvWriterV2, 2);
104+
105+
pcpMmvWriterV1.reset();
106+
assertStringsCount(pcpMmvWriterV1, 0);
107+
108+
pcpMmvWriterV2.reset();
109+
assertStringsCount(pcpMmvWriterV2, 0);
110+
}
111+
82112
private void assertMetric(String metricName, Matcher<String> expectedValue) throws Exception {
83113
String actual = pcpClient.getMetric(metricName);
84114
assertThat(actual, expectedValue);
85115
}
86116

117+
private void assertStringsCount(PcpMmvWriter writer, int expectedCount) throws NoSuchFieldException, IllegalAccessException {
118+
Field field = PcpMmvWriter.class.getDeclaredField("stringStore");
119+
field.setAccessible(true);
120+
PcpString.PcpStringStore stringStore = (PcpString.PcpStringStore) field.get(writer);
121+
assertEquals(expectedCount, stringStore.getStrings().size());
122+
}
123+
87124
private void waitForReload() throws InterruptedException {
88125
Thread.sleep(1000);
89126
}

0 commit comments

Comments
 (0)