From 7800187433ed9ba4cbbc6c8aebacf553f160c99e Mon Sep 17 00:00:00 2001 From: norman Date: Fri, 1 Jun 2012 09:25:59 +0200 Subject: [PATCH] Make sure calling ExecutionHandler.releaseExternalResource() does not lead to a dead-lock when calling from a ChannelEventRunnable. See #200 --- .../ChannelDownstreamEventRunnable.java | 9 +++++--- .../execution/ChannelEventRunnable.java | 22 ++++++++++++++++++- .../ChannelUpstreamEventRunnable.java | 7 +++--- .../handler/execution/ExecutionHandler.java | 9 ++++++-- .../MemoryAwareThreadPoolExecutor.java | 8 ++++++- .../OrderedMemoryAwareThreadPoolExecutor.java | 4 ++++ 6 files changed, 49 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/execution/ChannelDownstreamEventRunnable.java b/src/main/java/org/jboss/netty/handler/execution/ChannelDownstreamEventRunnable.java index 5d7bfc055b..9eb96e9796 100644 --- a/src/main/java/org/jboss/netty/handler/execution/ChannelDownstreamEventRunnable.java +++ b/src/main/java/org/jboss/netty/handler/execution/ChannelDownstreamEventRunnable.java @@ -16,6 +16,8 @@ package org.jboss.netty.handler.execution; +import java.util.concurrent.Executor; + import org.jboss.netty.channel.ChannelEvent; import org.jboss.netty.channel.ChannelHandlerContext; @@ -24,14 +26,15 @@ import org.jboss.netty.channel.ChannelHandlerContext; */ public class ChannelDownstreamEventRunnable extends ChannelEventRunnable { - public ChannelDownstreamEventRunnable(ChannelHandlerContext ctx, ChannelEvent e) { - super(ctx, e); + public ChannelDownstreamEventRunnable(ChannelHandlerContext ctx, ChannelEvent e, Executor executor) { + super(ctx, e, executor); } /** * Send the {@link ChannelEvent} downstream */ - public void run() { + @Override + public void runTask() { ctx.sendDownstream(e); } } diff --git a/src/main/java/org/jboss/netty/handler/execution/ChannelEventRunnable.java b/src/main/java/org/jboss/netty/handler/execution/ChannelEventRunnable.java index 5f3a2e4fdb..7635902c65 100644 --- a/src/main/java/org/jboss/netty/handler/execution/ChannelEventRunnable.java +++ b/src/main/java/org/jboss/netty/handler/execution/ChannelEventRunnable.java @@ -15,23 +15,28 @@ */ package org.jboss.netty.handler.execution; +import java.util.concurrent.Executor; + import org.jboss.netty.channel.ChannelEvent; import org.jboss.netty.channel.ChannelHandlerContext; import org.jboss.netty.util.EstimatableObjectWrapper; +import org.jboss.netty.util.internal.DeadLockProofWorker; public abstract class ChannelEventRunnable implements Runnable, EstimatableObjectWrapper { protected final ChannelHandlerContext ctx; protected final ChannelEvent e; int estimatedSize; + private final Executor executor; /** * Creates a {@link Runnable} which sends the specified {@link ChannelEvent} * upstream via the specified {@link ChannelHandlerContext}. */ - public ChannelEventRunnable(ChannelHandlerContext ctx, ChannelEvent e) { + public ChannelEventRunnable(ChannelHandlerContext ctx, ChannelEvent e, Executor executor) { this.ctx = ctx; this.e = e; + this.executor = executor; } /** @@ -52,4 +57,19 @@ public abstract class ChannelEventRunnable implements Runnable, EstimatableObjec public Object unwrap() { return e; } + + public final void run() { + try { + DeadLockProofWorker.PARENT.set(executor); + runTask(); + } finally { + DeadLockProofWorker.PARENT.remove(); + } + + } + + /** + * Run the task + */ + protected abstract void runTask(); } diff --git a/src/main/java/org/jboss/netty/handler/execution/ChannelUpstreamEventRunnable.java b/src/main/java/org/jboss/netty/handler/execution/ChannelUpstreamEventRunnable.java index 2ad4772ff3..50c8d1d5e7 100644 --- a/src/main/java/org/jboss/netty/handler/execution/ChannelUpstreamEventRunnable.java +++ b/src/main/java/org/jboss/netty/handler/execution/ChannelUpstreamEventRunnable.java @@ -32,15 +32,16 @@ public class ChannelUpstreamEventRunnable extends ChannelEventRunnable { * Creates a {@link Runnable} which sends the specified {@link ChannelEvent} * upstream via the specified {@link ChannelHandlerContext}. */ - public ChannelUpstreamEventRunnable(ChannelHandlerContext ctx, ChannelEvent e) { - super(ctx, e); + public ChannelUpstreamEventRunnable(ChannelHandlerContext ctx, ChannelEvent e, Executor executor) { + super(ctx, e, executor); } /** * Sends the event upstream. */ - public void run() { + @Override + protected void runTask() { ctx.sendUpstream(e); } } diff --git a/src/main/java/org/jboss/netty/handler/execution/ExecutionHandler.java b/src/main/java/org/jboss/netty/handler/execution/ExecutionHandler.java index 3427f6169f..06489acf7d 100644 --- a/src/main/java/org/jboss/netty/handler/execution/ExecutionHandler.java +++ b/src/main/java/org/jboss/netty/handler/execution/ExecutionHandler.java @@ -169,7 +169,12 @@ public class ExecutionHandler implements ChannelUpstreamHandler, ChannelDownstre public void handleUpstream( ChannelHandlerContext context, ChannelEvent e) throws Exception { if (handleUpstream) { - executor.execute(new ChannelUpstreamEventRunnable(context, e)); + try { + executor.execute(new ChannelUpstreamEventRunnable(context, e, executor)); + + } finally { + + } } else { context.sendUpstream(e); } @@ -180,7 +185,7 @@ public class ExecutionHandler implements ChannelUpstreamHandler, ChannelDownstre // check if the read was suspend if (!handleReadSuspend(ctx, e)) { if (handleDownstream) { - executor.execute(new ChannelDownstreamEventRunnable(ctx, e)); + executor.execute(new ChannelDownstreamEventRunnable(ctx, e, executor)); } else { ctx.sendDownstream(e); } diff --git a/src/main/java/org/jboss/netty/handler/execution/MemoryAwareThreadPoolExecutor.java b/src/main/java/org/jboss/netty/handler/execution/MemoryAwareThreadPoolExecutor.java index 96e02d858c..90d684e6f7 100644 --- a/src/main/java/org/jboss/netty/handler/execution/MemoryAwareThreadPoolExecutor.java +++ b/src/main/java/org/jboss/netty/handler/execution/MemoryAwareThreadPoolExecutor.java @@ -45,6 +45,7 @@ import org.jboss.netty.logging.InternalLoggerFactory; import org.jboss.netty.util.DefaultObjectSizeEstimator; import org.jboss.netty.util.ObjectSizeEstimator; import org.jboss.netty.util.internal.ConcurrentIdentityHashMap; +import org.jboss.netty.util.internal.DeadLockProofWorker; import org.jboss.netty.util.internal.QueueFactory; import org.jboss.netty.util.internal.SharedResourceMisuseDetector; @@ -441,7 +442,12 @@ public class MemoryAwareThreadPoolExecutor extends ThreadPoolExecutor { * Executes the specified task without maintaining the event order. */ protected final void doUnorderedExecute(Runnable task) { - super.execute(task); + try { + DeadLockProofWorker.PARENT.set(this); + super.execute(task); + } finally { + DeadLockProofWorker.PARENT.remove(); + } } @Override diff --git a/src/main/java/org/jboss/netty/handler/execution/OrderedMemoryAwareThreadPoolExecutor.java b/src/main/java/org/jboss/netty/handler/execution/OrderedMemoryAwareThreadPoolExecutor.java index 7b186ef459..3385bf1e5f 100644 --- a/src/main/java/org/jboss/netty/handler/execution/OrderedMemoryAwareThreadPoolExecutor.java +++ b/src/main/java/org/jboss/netty/handler/execution/OrderedMemoryAwareThreadPoolExecutor.java @@ -31,6 +31,7 @@ import org.jboss.netty.channel.ChannelState; import org.jboss.netty.channel.ChannelStateEvent; import org.jboss.netty.util.ObjectSizeEstimator; import org.jboss.netty.util.internal.ConcurrentIdentityWeakKeyHashMap; +import org.jboss.netty.util.internal.DeadLockProofWorker; import org.jboss.netty.util.internal.QueueFactory; /** @@ -302,6 +303,7 @@ public class OrderedMemoryAwareThreadPoolExecutor extends acquired = true; try { Thread thread = Thread.currentThread(); + DeadLockProofWorker.PARENT.set(OrderedMemoryAwareThreadPoolExecutor.this); for (;;) { final Runnable task = tasks.poll(); // if the task is null we should exit the loop @@ -323,6 +325,8 @@ public class OrderedMemoryAwareThreadPoolExecutor extends } } } finally { + DeadLockProofWorker.PARENT.remove(); + // set it back to not running isRunning.set(false); }