From a0ed6ec06ca605ed39b601355daedace93828032 Mon Sep 17 00:00:00 2001 From: Xiaoyan Lin Date: Tue, 8 May 2018 11:09:16 -0700 Subject: [PATCH] Fix the error message in ReferenceCounted.release (#7921) Motivation: When a buffer is over-released, the current error message of `IllegalReferenceCountException` is `refCnt: XXX, increment: XXX`, which is confusing. The correct message should be `refCnt: XXX, decrement: XXX`. Modifications: Pass `-decrement` to create `IllegalReferenceCountException`. Result: The error message will be `refCnt: XXX, decrement: XXX` when a buffer is over-released. --- .../buffer/AbstractReferenceCountedByteBuf.java | 2 +- .../buffer/AbstractReferenceCountedByteBufTest.java | 13 +++++++++++++ .../io/netty/util/AbstractReferenceCounted.java | 2 +- .../io/netty/util/AbstractReferenceCountedTest.java | 13 +++++++++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java index d3058c0fb6..d624d855f4 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java @@ -97,7 +97,7 @@ public abstract class AbstractReferenceCountedByteBuf extends AbstractByteBuf { } else if (oldRef < decrement || oldRef - decrement > oldRef) { // Ensure we don't over-release, and avoid underflow. refCntUpdater.getAndAdd(this, decrement); - throw new IllegalReferenceCountException(oldRef, decrement); + throw new IllegalReferenceCountException(oldRef, -decrement); } return false; } diff --git a/buffer/src/test/java/io/netty/buffer/AbstractReferenceCountedByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractReferenceCountedByteBufTest.java index 48fe415332..dcc25b20cb 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractReferenceCountedByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractReferenceCountedByteBufTest.java @@ -29,6 +29,7 @@ import java.nio.channels.ScatteringByteChannel; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class AbstractReferenceCountedByteBufTest { @@ -55,6 +56,18 @@ public class AbstractReferenceCountedByteBufTest { referenceCounted.release(Integer.MAX_VALUE); } + @Test + public void testReleaseErrorMessage() { + AbstractReferenceCountedByteBuf referenceCounted = newReferenceCounted(); + assertTrue(referenceCounted.release()); + try { + referenceCounted.release(1); + fail("IllegalReferenceCountException didn't occur"); + } catch (IllegalReferenceCountException e) { + assertEquals("refCnt: 0, decrement: 1", e.getMessage()); + } + } + @Test(expected = IllegalReferenceCountException.class) public void testRetainResurrect() { AbstractReferenceCountedByteBuf referenceCounted = newReferenceCounted(); diff --git a/common/src/main/java/io/netty/util/AbstractReferenceCounted.java b/common/src/main/java/io/netty/util/AbstractReferenceCounted.java index 5fae9dca11..fe20f92a2e 100644 --- a/common/src/main/java/io/netty/util/AbstractReferenceCounted.java +++ b/common/src/main/java/io/netty/util/AbstractReferenceCounted.java @@ -84,7 +84,7 @@ public abstract class AbstractReferenceCounted implements ReferenceCounted { } else if (oldRef < decrement || oldRef - decrement > oldRef) { // Ensure we don't over-release, and avoid underflow. refCntUpdater.getAndAdd(this, decrement); - throw new IllegalReferenceCountException(oldRef, decrement); + throw new IllegalReferenceCountException(oldRef, -decrement); } return false; } diff --git a/common/src/test/java/io/netty/util/AbstractReferenceCountedTest.java b/common/src/test/java/io/netty/util/AbstractReferenceCountedTest.java index e004e5df7b..5af113490f 100644 --- a/common/src/test/java/io/netty/util/AbstractReferenceCountedTest.java +++ b/common/src/test/java/io/netty/util/AbstractReferenceCountedTest.java @@ -19,6 +19,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class AbstractReferenceCountedTest { @@ -45,6 +46,18 @@ public class AbstractReferenceCountedTest { referenceCounted.release(Integer.MAX_VALUE); } + @Test + public void testReleaseErrorMessage() { + AbstractReferenceCounted referenceCounted = newReferenceCounted(); + assertTrue(referenceCounted.release()); + try { + referenceCounted.release(1); + fail("IllegalReferenceCountException didn't occur"); + } catch (IllegalReferenceCountException e) { + assertEquals("refCnt: 0, decrement: 1", e.getMessage()); + } + } + @Test(expected = IllegalReferenceCountException.class) public void testRetainResurrect() { AbstractReferenceCounted referenceCounted = newReferenceCounted();