From 2457f386d8873d4b401571eb676c0b8b84288c76 Mon Sep 17 00:00:00 2001 From: Shixiong Zhu Date: Fri, 6 Jan 2017 13:12:43 -0800 Subject: [PATCH] Set `prev` to null when setting `cursor` to `head` in `scavengeSome`. Motivation: `scavengeSome()` has a corner case: when setting `cursor` to `head`, `this.prev` may point to the tail of the `WeakOrderQueue` linked list. Then it's possible that the following while loop will link the tail to the head, and cause endless loop. I made a reproducer in https://github.com/zsxwing/netty/commit/36522e7b72d61af8e4455162b2e33ce42194774d . The unit test will just run forever. Unfortunately, I cannot change it to a unit test because it needs to add some codes to `scavengeSome` to control the execution flow. Modification: Set `prev` to null when setting `cursor` to `head` in `scavengeSome` Result: Fixes #6153. --- common/src/main/java/io/netty/util/Recycler.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/io/netty/util/Recycler.java b/common/src/main/java/io/netty/util/Recycler.java index 30425f7947..2164ac3127 100644 --- a/common/src/main/java/io/netty/util/Recycler.java +++ b/common/src/main/java/io/netty/util/Recycler.java @@ -479,16 +479,19 @@ public abstract class Recycler { } boolean scavengeSome() { + WeakOrderQueue prev; WeakOrderQueue cursor = this.cursor; if (cursor == null) { + prev = null; cursor = head; if (cursor == null) { return false; } + } else { + prev = this.prev; } boolean success = false; - WeakOrderQueue prev = this.prev; do { if (cursor.transfer(this)) { success = true;