From b876bd8cec3d6f403a577e0094e992b8ab0a9687 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 13 Nov 2008 14:11:59 +0000 Subject: [PATCH] Added a potential fix for infinite loop in LinkedTransferQueue.clean() --- .../jboss/netty/util/LinkedTransferQueue.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jboss/netty/util/LinkedTransferQueue.java b/src/main/java/org/jboss/netty/util/LinkedTransferQueue.java index 766218fc5e..87548bc44f 100644 --- a/src/main/java/org/jboss/netty/util/LinkedTransferQueue.java +++ b/src/main/java/org/jboss/netty/util/LinkedTransferQueue.java @@ -95,6 +95,8 @@ public class LinkedTransferQueue extends AbstractQueue implements Blocking /** The number of CPUs, for spin control */ private static final int NCPUS = Runtime.getRuntime().availableProcessors(); + private static final QNode UNDEFINED = new QNode(null, false); + /** * The number of times to spin before blocking in timed waits. * The value is empirically derived -- it works well across a @@ -345,7 +347,7 @@ public class LinkedTransferQueue extends AbstractQueue implements Blocking * Gets rid of cancelled node s with original predecessor pred. * @return null (to simplify use by callers) */ - Object clean(QNode pred, QNode s) { + Object clean(final QNode pred, final QNode s) { Thread w = s.waiter; if (w != null) { // Wake up thread s.waiter = null; @@ -354,6 +356,7 @@ public class LinkedTransferQueue extends AbstractQueue implements Blocking } } + QNode olddp = UNDEFINED; for (;;) { if (pred.next != s) { return null; @@ -382,6 +385,8 @@ public class LinkedTransferQueue extends AbstractQueue implements Blocking return null; } } + + boolean stateUnchanged = true; QNode dp = cleanMe.get(); if (dp != null) { // Try unlinking previous cancelled node QNode d = dp.next; @@ -389,15 +394,22 @@ public class LinkedTransferQueue extends AbstractQueue implements Blocking if (d == null || // d is gone or d == dp || // d is off list or d.get() != d || // d not cancelled or - d != t && // d not tail and - (dn = d.next) != null && // has successor + d != t && // d not tail and + (dn = d.next) != null && // has successor dn != d && // that is on list dp.casNext(d, dn)) { cleanMe.compareAndSet(dp, null); + stateUnchanged = false; } if (dp == pred) { return null; // s is already saved node } + + if (stateUnchanged && olddp == dp) { + return null; // infinite loop expected - bail out + } else { + olddp = dp; + } } else if (cleanMe.compareAndSet(null, pred)) { return null; // Postpone cleaning s