From ddf7607ad197bb2393b85eb90314fa3f422a0894 Mon Sep 17 00:00:00 2001 From: Jerry Lee Date: Tue, 1 Apr 2014 01:03:45 +0800 Subject: [PATCH] fix idempotency #36 --- .../com/alibaba/mtc/MtContextCallable.java | 49 ++++++++++++++++-- .../com/alibaba/mtc/MtContextRunnable.java | 50 +++++++++++++++--- .../com/alibaba/mtc/MtContextTimerTask.java | 51 ++++++++++++++++--- .../agent/MtContextTransformer.java | 4 +- .../alibaba/mtc/MtContextCallableTest.java | 6 +-- 5 files changed, 138 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/alibaba/mtc/MtContextCallable.java b/src/main/java/com/alibaba/mtc/MtContextCallable.java index 66e68f16..1127f320 100644 --- a/src/main/java/com/alibaba/mtc/MtContextCallable.java +++ b/src/main/java/com/alibaba/mtc/MtContextCallable.java @@ -30,9 +30,9 @@ public final class MtContextCallable implements Callable { private MtContextCallable(Callable callable, boolean releaseMtContextAfterCall) { + this.copiedRef = new AtomicReference, Object>>(MtContextThreadLocal.copy()); this.callable = callable; this.releaseMtContextAfterCall = releaseMtContextAfterCall; - this.copiedRef = new AtomicReference, Object>>(MtContextThreadLocal.copy()); } /** @@ -69,6 +69,7 @@ public final class MtContextCallable implements Callable { return get(callable, false); } + /** * Factory method, wrapper input {@link Callable} to {@link MtContextCallable}. *

@@ -79,12 +80,31 @@ public final class MtContextCallable implements Callable { * @return Wrapped {@link Callable} */ public static MtContextCallable get(Callable callable, boolean releaseMtContextAfterCall) { + return get(callable, releaseMtContextAfterCall, false); + } + + /** + * Factory method, wrapper input {@link Callable} to {@link MtContextCallable}. + *

+ * This method is idempotent. + * + * @param callable input {@link Callable} + * @param releaseMtContextAfterCall release MtContext after run, avoid memory leak even if {@link MtContextRunnable} is referred. + * @param idempotent is idempotent or not. {@code true} will cover up bug! DO NOT set, only when you why. + * @return Wrapped {@link Callable} + */ + public static MtContextCallable get(Callable callable, boolean releaseMtContextAfterCall, boolean idempotent) { if (null == callable) { return null; } - if (callable instanceof MtContextCallable) { // avoid redundant decoration, and ensure idempotency - throw new IllegalStateException("Already MtContextCallable!"); + if (callable instanceof MtContextCallable) { + if (idempotent) { + // avoid redundant decoration, and ensure idempotency + return (MtContextCallable) callable; + } else { + throw new IllegalStateException("Already MtContextCallable!"); + } } return new MtContextCallable(callable, releaseMtContextAfterCall); } @@ -96,7 +116,7 @@ public final class MtContextCallable implements Callable { * @return Wrapped {@link Callable} */ public static List> gets(Collection> tasks) { - return gets(tasks, false); + return gets(tasks, false, false); } /** @@ -112,7 +132,26 @@ public final class MtContextCallable implements Callable { } List> copy = new ArrayList>(); for (Callable task : tasks) { - copy.add(MtContextCallable.get(task, releaseMtContextAfterCall)); + copy.add(MtContextCallable.get(task, releaseMtContextAfterCall, false)); + } + return copy; + } + + /** + * wrapper input {@link Callable} Collection to {@link MtContextCallable} Collection. + * + * @param tasks task to be wrapped + * @param releaseMtContextAfterCall release MtContext after run, avoid memory leak even if {@link MtContextRunnable} is referred. + * @param idempotent is idempotent or not. {@code true} will cover up bug! DO NOT set, only when you why. + * @return Wrapped {@link Callable} + */ + public static List> gets(Collection> tasks, boolean releaseMtContextAfterCall, boolean idempotent) { + if (null == tasks) { + return null; + } + List> copy = new ArrayList>(); + for (Callable task : tasks) { + copy.add(MtContextCallable.get(task, releaseMtContextAfterCall, idempotent)); } return copy; } diff --git a/src/main/java/com/alibaba/mtc/MtContextRunnable.java b/src/main/java/com/alibaba/mtc/MtContextRunnable.java index 590b5fa4..633306b3 100644 --- a/src/main/java/com/alibaba/mtc/MtContextRunnable.java +++ b/src/main/java/com/alibaba/mtc/MtContextRunnable.java @@ -26,9 +26,9 @@ public final class MtContextRunnable implements Runnable { private final boolean releaseMtContextAfterRun; private MtContextRunnable(Runnable runnable, boolean releaseMtContextAfterRun) { + this.copiedRef = new AtomicReference, Object>>(MtContextThreadLocal.copy()); this.runnable = runnable; this.releaseMtContextAfterRun = releaseMtContextAfterRun; - this.copiedRef = new AtomicReference, Object>>(MtContextThreadLocal.copy()); } /** @@ -62,7 +62,7 @@ public final class MtContextRunnable implements Runnable { * @return Wrapped {@link Runnable} */ public static MtContextRunnable get(Runnable runnable) { - return get(runnable, false); + return get(runnable, false, false); } /** @@ -75,12 +75,31 @@ public final class MtContextRunnable implements Runnable { * @return Wrapped {@link Runnable} */ public static MtContextRunnable get(Runnable runnable, boolean releaseMtContextAfterRun) { + return get(runnable, releaseMtContextAfterRun, false); + } + + /** + * Factory method, wrapper input {@link Runnable} to {@link MtContextRunnable}. + *

+ * This method is idempotent. + * + * @param runnable input {@link Runnable} + * @param releaseMtContextAfterRun release MtContext after run, avoid memory leak even if {@link MtContextRunnable} is referred. + * @param idempotent is idempotent or not. {@code true} will cover up bug! DO NOT set, only when you why. + * @return Wrapped {@link Runnable} + */ + public static MtContextRunnable get(Runnable runnable, boolean releaseMtContextAfterRun, boolean idempotent) { if (null == runnable) { return null; } - if (runnable instanceof MtContextRunnable) { // avoid redundant decoration, and ensure idempotency - throw new IllegalStateException("Already MtContextRunnable!"); + if (runnable instanceof MtContextRunnable) { + if (idempotent) { + // avoid redundant decoration, and ensure idempotency + return (MtContextRunnable) runnable; + } else { + throw new IllegalStateException("Already MtContextRunnable!"); + } } return new MtContextRunnable(runnable, releaseMtContextAfterRun); } @@ -92,7 +111,7 @@ public final class MtContextRunnable implements Runnable { * @return wrapped tasks */ public static List gets(Collection tasks) { - return gets(tasks, false); + return gets(tasks, false, false); } /** @@ -108,7 +127,26 @@ public final class MtContextRunnable implements Runnable { } List copy = new ArrayList(); for (Runnable task : tasks) { - copy.add(MtContextRunnable.get(task, releaseMtContextAfterRun)); + copy.add(MtContextRunnable.get(task, releaseMtContextAfterRun, false)); + } + return copy; + } + + /** + * wrapper input {@link Runnable} Collection to {@link MtContextRunnable} Collection. + * + * @param tasks task to be wrapped + * @param releaseMtContextAfterRun release MtContext after run, avoid memory leak even if {@link MtContextRunnable} is referred. + * @param idempotent is idempotent or not. {@code true} will cover up bug! DO NOT set, only when you why. + * @return wrapped tasks + */ + public static List gets(Collection tasks, boolean releaseMtContextAfterRun, boolean idempotent) { + if (null == tasks) { + return null; + } + List copy = new ArrayList(); + for (Runnable task : tasks) { + copy.add(MtContextRunnable.get(task, releaseMtContextAfterRun, idempotent)); } return copy; } diff --git a/src/main/java/com/alibaba/mtc/MtContextTimerTask.java b/src/main/java/com/alibaba/mtc/MtContextTimerTask.java index c5bd91d7..e5d6f650 100644 --- a/src/main/java/com/alibaba/mtc/MtContextTimerTask.java +++ b/src/main/java/com/alibaba/mtc/MtContextTimerTask.java @@ -2,6 +2,7 @@ package com.alibaba.mtc; import java.util.Map; import java.util.TimerTask; +import java.util.concurrent.atomic.AtomicReference; /** * {@link MtContextTimerTask} decorate {@link TimerTask}, so as to get {@link MtContextThreadLocal} @@ -21,12 +22,14 @@ import java.util.TimerTask; */ @Deprecated public final class MtContextTimerTask extends TimerTask { - private final Map, Object> copied; + private final AtomicReference, Object>> copiedRef; private final TimerTask timerTask; + private final boolean releaseMtContextAfterRun; - private MtContextTimerTask(TimerTask timerTask) { - copied = MtContextThreadLocal.copy(); + private MtContextTimerTask(TimerTask timerTask, boolean releaseMtContextAfterRun) { + this.copiedRef = new AtomicReference, Object>>(MtContextThreadLocal.copy()); this.timerTask = timerTask; + this.releaseMtContextAfterRun = releaseMtContextAfterRun; } /** @@ -35,7 +38,11 @@ public final class MtContextTimerTask extends TimerTask { @Override public void run() { // backup MtContext + Map, Object> copied = copiedRef.get(); Map, Object> backup = MtContextThreadLocal.backupAndSet(copied); + if (copied == null || releaseMtContextAfterRun && !copiedRef.compareAndSet(copied, null)) { + throw new IllegalStateException("MtContext is released!"); + } try { timerTask.run(); } finally { @@ -62,13 +69,45 @@ public final class MtContextTimerTask extends TimerTask { * @return Wrapped {@link TimerTask} */ public static MtContextTimerTask get(TimerTask timerTask) { + return get(timerTask, false, false); + } + + /** + * Factory method, wrapper input {@link Runnable} to {@link MtContextTimerTask}. + *

+ * This method is idempotent. + * + * @param timerTask input {@link TimerTask} + * @param releaseMtContextAfterRun release MtContext after run, avoid memory leak even if {@link MtContextRunnable} is referred. + * @return Wrapped {@link TimerTask} + */ + public static MtContextTimerTask get(TimerTask timerTask, boolean releaseMtContextAfterRun) { + return get(timerTask, releaseMtContextAfterRun, false); + } + + /** + * Factory method, wrapper input {@link Runnable} to {@link MtContextTimerTask}. + *

+ * This method is idempotent. + * + * @param timerTask input {@link TimerTask} + * @param releaseMtContextAfterRun release MtContext after run, avoid memory leak even if {@link MtContextRunnable} is referred. + * @param idempotent is idempotent or not. {@code true} will cover up bug! DO NOT set, only when you why. + * @return Wrapped {@link TimerTask} + */ + public static MtContextTimerTask get(TimerTask timerTask, boolean releaseMtContextAfterRun, boolean idempotent) { if (null == timerTask) { return null; } - if (timerTask instanceof MtContextTimerTask) { // avoid redundant decoration, and ensure idempotency - throw new IllegalStateException("Already MtContextTimerTask!"); + if (timerTask instanceof MtContextTimerTask) { + if (idempotent) { + // avoid redundant decoration, and ensure idempotency + return (MtContextTimerTask) timerTask; + } else { + throw new IllegalStateException("Already MtContextTimerTask!"); + } } - return new MtContextTimerTask(timerTask); + return new MtContextTimerTask(timerTask, false); } } diff --git a/src/main/java/com/alibaba/mtc/threadpool/agent/MtContextTransformer.java b/src/main/java/com/alibaba/mtc/threadpool/agent/MtContextTransformer.java index 5b6b3a48..b12b533c 100644 --- a/src/main/java/com/alibaba/mtc/threadpool/agent/MtContextTransformer.java +++ b/src/main/java/com/alibaba/mtc/threadpool/agent/MtContextTransformer.java @@ -115,11 +115,11 @@ public class MtContextTransformer implements ClassFileTransformer { for (int i = 0; i < parameterTypes.length; i++) { CtClass paraType = parameterTypes[i]; if (RUNNABLE_CLASS_NAME.equals(paraType.getName())) { - String code = String.format("$%d = %s.get($%d);", i + 1, MT_CONTEXT_RUNNABLE_CLASS_NAME, i + 1); + String code = String.format("$%d = %s.get($%d, false, true);", i + 1, MT_CONTEXT_RUNNABLE_CLASS_NAME, i + 1); logger.info("insert code before method " + method + " of class " + method.getDeclaringClass().getName() + ": " + code); insertCode.append(code); } else if (CALLABLE_CLASS_NAME.equals(paraType.getName())) { - String code = String.format("$%d = %s.get($%d);", i + 1, MT_CONTEXT_CALLABLE_CLASS_NAME, i + 1); + String code = String.format("$%d = %s.get($%d, false, true);", i + 1, MT_CONTEXT_CALLABLE_CLASS_NAME, i + 1); logger.info("insert code before method " + method + " of class " + method.getDeclaringClass().getName() + ": " + code); insertCode.append(code); } diff --git a/src/test/java/com/alibaba/mtc/MtContextCallableTest.java b/src/test/java/com/alibaba/mtc/MtContextCallableTest.java index aa2c0fda..f9d9a8c4 100644 --- a/src/test/java/com/alibaba/mtc/MtContextCallableTest.java +++ b/src/test/java/com/alibaba/mtc/MtContextCallableTest.java @@ -206,12 +206,12 @@ public class MtContextCallableTest { @Test public void test_gets() throws Exception { - Call call1 = new Call("1", null); - Call call2 = new Call("1", null); + Callable call1 = new Call("1", null); + Callable call2 = new Call("1", null); Callable call3 = new Call("1", null); List> callList = MtContextCallable.gets( - Arrays.>asList(call1, call2, null, call3)); + Arrays.asList(call1, call2, null, call3)); assertEquals(4, callList.size()); assertThat(callList.get(0), instanceOf(MtContextCallable.class)); -- GitLab