From 621df7c978aa18e6e2336289ca6b462d29790f75 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Fri, 30 Jun 2017 10:33:29 +0200 Subject: [PATCH] Add ByteBuf leak detection @Rule This commit introduces a JUnit rule that detects ByteBuf leaks in subclasses of AbstractDataBufferAllocatingTestCase. --- .../io/buffer/NettyDataBufferFactory.java | 8 +++- .../core/codec/DataBufferDecoderTests.java | 6 +-- .../core/codec/DataBufferEncoderTests.java | 6 +-- .../AbstractDataBufferAllocatingTestCase.java | 44 +++++++++++++++++-- .../core/io/buffer/DataBufferTests.java | 4 +- 5 files changed, 56 insertions(+), 12 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/io/buffer/NettyDataBufferFactory.java b/spring-core/src/main/java/org/springframework/core/io/buffer/NettyDataBufferFactory.java index a88bd01ace..ba1ad13d70 100644 --- a/spring-core/src/main/java/org/springframework/core/io/buffer/NettyDataBufferFactory.java +++ b/spring-core/src/main/java/org/springframework/core/io/buffer/NettyDataBufferFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -49,6 +49,12 @@ public class NettyDataBufferFactory implements DataBufferFactory { this.byteBufAllocator = byteBufAllocator; } + /** + * Return the {@code ByteBufAllocator} used by this factory. + */ + public ByteBufAllocator getByteBufAllocator() { + return this.byteBufAllocator; + } @Override public NettyDataBuffer allocateBuffer() { diff --git a/spring-core/src/test/java/org/springframework/core/codec/DataBufferDecoderTests.java b/spring-core/src/test/java/org/springframework/core/codec/DataBufferDecoderTests.java index 25a4fd8890..704db7135f 100644 --- a/spring-core/src/test/java/org/springframework/core/codec/DataBufferDecoderTests.java +++ b/spring-core/src/test/java/org/springframework/core/codec/DataBufferDecoderTests.java @@ -28,9 +28,7 @@ import org.springframework.core.io.buffer.AbstractDataBufferAllocatingTestCase; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.util.MimeTypeUtils; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; /** * @author Sebastien Deleuze @@ -59,5 +57,7 @@ public class DataBufferDecoderTests extends AbstractDataBufferAllocatingTestCase null, Collections.emptyMap()); assertSame(source, output); + + release(fooBuffer, barBuffer); } } diff --git a/spring-core/src/test/java/org/springframework/core/codec/DataBufferEncoderTests.java b/spring-core/src/test/java/org/springframework/core/codec/DataBufferEncoderTests.java index 7e7ba5de69..dfa788f88d 100644 --- a/spring-core/src/test/java/org/springframework/core/codec/DataBufferEncoderTests.java +++ b/spring-core/src/test/java/org/springframework/core/codec/DataBufferEncoderTests.java @@ -28,9 +28,7 @@ import org.springframework.core.io.buffer.AbstractDataBufferAllocatingTestCase; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.util.MimeTypeUtils; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; /** * @author Sebastien Deleuze @@ -63,6 +61,8 @@ public class DataBufferEncoderTests extends AbstractDataBufferAllocatingTestCase null, Collections.emptyMap()); assertSame(source, output); + + release(fooBuffer, barBuffer); } } diff --git a/spring-core/src/test/java/org/springframework/core/io/buffer/AbstractDataBufferAllocatingTestCase.java b/spring-core/src/test/java/org/springframework/core/io/buffer/AbstractDataBufferAllocatingTestCase.java index 272af8c01d..b4417b0cec 100644 --- a/spring-core/src/test/java/org/springframework/core/io/buffer/AbstractDataBufferAllocatingTestCase.java +++ b/spring-core/src/test/java/org/springframework/core/io/buffer/AbstractDataBufferAllocatingTestCase.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,16 +18,22 @@ package org.springframework.core.io.buffer; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.List; import java.util.function.Consumer; +import io.netty.buffer.ByteBufAllocator; +import io.netty.buffer.PoolArenaMetric; import io.netty.buffer.PooledByteBufAllocator; +import io.netty.buffer.PooledByteBufAllocatorMetric; import io.netty.buffer.UnpooledByteBufAllocator; +import org.junit.Rule; +import org.junit.rules.Verifier; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.springframework.core.io.buffer.support.DataBufferTestUtils; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; /** * @author Arjen Poutsma @@ -43,14 +49,18 @@ public abstract class AbstractDataBufferAllocatingTestCase { return new Object[][] { {new NettyDataBufferFactory(new UnpooledByteBufAllocator(true))}, {new NettyDataBufferFactory(new UnpooledByteBufAllocator(false))}, - {new NettyDataBufferFactory(new PooledByteBufAllocator(true))}, - {new NettyDataBufferFactory(new PooledByteBufAllocator(false))}, + // disable caching for reliable leak detection, see https://github.com/netty/netty/issues/5275 + {new NettyDataBufferFactory(new PooledByteBufAllocator(true, 1, 1, 8192, 11, 0, 0, 0, true))}, + {new NettyDataBufferFactory(new PooledByteBufAllocator(false, 1, 1, 8192, 11, 0, 0, 0, true))}, {new DefaultDataBufferFactory(true)}, {new DefaultDataBufferFactory(false)} }; } + @Rule + public final Verifier leakDetector = new LeakDetector(); + protected DataBuffer createDataBuffer(int capacity) { return this.bufferFactory.allocateBuffer(capacity); } @@ -75,4 +85,30 @@ public abstract class AbstractDataBufferAllocatingTestCase { }; } + + private class LeakDetector extends Verifier { + + @Override + protected void verify() throws Throwable { + if (bufferFactory instanceof NettyDataBufferFactory) { + ByteBufAllocator byteBufAllocator = + ((NettyDataBufferFactory) bufferFactory).getByteBufAllocator(); + if (byteBufAllocator instanceof PooledByteBufAllocator) { + PooledByteBufAllocator pooledByteBufAllocator = + (PooledByteBufAllocator) byteBufAllocator; + PooledByteBufAllocatorMetric metric = pooledByteBufAllocator.metric(); + long allocations = calculateAllocations(metric.directArenas()) + + calculateAllocations(metric.heapArenas()); + assertTrue("ByteBuf leak detected: " + allocations + + " allocations were not released", allocations == 0); + } + } + } + + private long calculateAllocations(List metrics) { + return metrics.stream().mapToLong(PoolArenaMetric::numActiveAllocations).sum(); + } + + } + } diff --git a/spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java b/spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java index 8f0bbfd487..cd0d0aeb7e 100644 --- a/spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -280,6 +280,8 @@ public class DataBufferTests extends AbstractDataBufferAllocatingTestCase { bytes = new byte[9]; buffer.read(bytes); assertArrayEquals(" World!!!".getBytes(StandardCharsets.UTF_8), bytes); + + release(buffer); }