From 423701d4d7c591baad17191be22f7f49470a0342 Mon Sep 17 00:00:00 2001 From: gnembon Date: Tue, 17 Dec 2019 11:58:35 -0400 Subject: [PATCH 1/3] Revert "threading testing" This reverts commit 9ba573af --- ...nager_scarpetThreadingProtectionMixin.java | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 src/main/java/carpet/mixins/ServerChunkManager_scarpetThreadingProtectionMixin.java diff --git a/src/main/java/carpet/mixins/ServerChunkManager_scarpetThreadingProtectionMixin.java b/src/main/java/carpet/mixins/ServerChunkManager_scarpetThreadingProtectionMixin.java deleted file mode 100644 index 897b53b7..00000000 --- a/src/main/java/carpet/mixins/ServerChunkManager_scarpetThreadingProtectionMixin.java +++ /dev/null @@ -1,39 +0,0 @@ -package carpet.mixins; - -import carpet.script.exception.InternalExpressionException; -import carpet.script.value.ThreadValue; -import net.minecraft.server.world.ServerChunkManager; -import net.minecraft.world.chunk.Chunk; -import net.minecraft.world.chunk.ChunkStatus; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.Redirect; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; - -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Executor; -import java.util.function.Supplier; - -@Mixin(ServerChunkManager.class) -public class ServerChunkManager_scarpetThreadingProtectionMixin -{ - @Redirect(method = "getChunk(IILnet/minecraft/world/chunk/ChunkStatus;Z)Lnet/minecraft/world/chunk/Chunk;", at = @At( - value = "INVOKE", - target = "Ljava/util/concurrent/CompletableFuture;join()Ljava/lang/Object;", - ordinal = 0 - )) - private Object checkJoiningThreads(CompletableFuture completableFuture) - { - synchronized (ThreadValue.waitingTreads) - { - if (ThreadValue.waitingTreads.containsKey(Thread.currentThread())) - throw new InternalExpressionException("Cannot join/wait on world accessing thread"); - ThreadValue.waitingTreads.put(Thread.currentThread(), 1); - } - Chunk res = completableFuture.join(); - ThreadValue.waitingTreads.remove(Thread.currentThread()); - return res; - - } -} From 3295ca2af98db53691ed9c016bdb1203ead775e0 Mon Sep 17 00:00:00 2001 From: gnembon Date: Tue, 17 Dec 2019 11:59:23 -0400 Subject: [PATCH 2/3] Revert "threading testing" This reverts commit 9ba573af --- .../java/carpet/script/CarpetExpression.java | 5 -- .../java/carpet/script/value/ThreadValue.java | 55 +++++-------------- src/main/resources/carpet.mixins.json | 1 - 3 files changed, 14 insertions(+), 47 deletions(-) diff --git a/src/main/java/carpet/script/CarpetExpression.java b/src/main/java/carpet/script/CarpetExpression.java index 8596f64b..e5a27ac0 100644 --- a/src/main/java/carpet/script/CarpetExpression.java +++ b/src/main/java/carpet/script/CarpetExpression.java @@ -944,18 +944,14 @@ public class CarpetExpression this.expr.addLazyFunction("set", -1, (c, t, lv) -> { - CarpetSettings.LOG.error("starting"); CarpetContext cc = (CarpetContext)c; ServerWorld world = cc.s.getWorld(); if (lv.size() < 2 || lv.size() % 2 == 1) throw new InternalExpressionException("'set' should have at least 2 params and odd attributes"); BlockValue.LocatorResult targetLocator = BlockValue.fromParams(cc, lv, 0); BlockValue.LocatorResult sourceLocator = BlockValue.fromParams(cc, lv, targetLocator.offset, true); - CarpetSettings.LOG.error("dafuq"); BlockState sourceBlockState = sourceLocator.block.getBlockState(); - CarpetSettings.LOG.error("finish dafuq"); BlockState targetBlockState = world.getBlockState(targetLocator.block.getPos()); - CarpetSettings.LOG.error("finish dafuq 2"); if (sourceLocator.offset < lv.size()) { StateFactory states = sourceBlockState.getBlock().getStateFactory(); @@ -969,7 +965,6 @@ public class CarpetExpression sourceBlockState = setProperty(property, paramString, paramValue, sourceBlockState); } } - CarpetSettings.LOG.error("finish properties got"); CompoundTag data = sourceLocator.block.getData(); diff --git a/src/main/java/carpet/script/value/ThreadValue.java b/src/main/java/carpet/script/value/ThreadValue.java index 6c5409bd..382cd6b2 100644 --- a/src/main/java/carpet/script/value/ThreadValue.java +++ b/src/main/java/carpet/script/value/ThreadValue.java @@ -1,6 +1,5 @@ package carpet.script.value; -import carpet.CarpetSettings; import carpet.script.Context; import carpet.script.Expression; import carpet.script.Tokenizer; @@ -10,7 +9,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; import java.util.concurrent.ThreadPoolExecutor; @@ -19,13 +17,9 @@ import java.util.function.Supplier; public class ThreadValue extends Value { private CompletableFuture taskFuture; - //public static ThreadLocal joining = ThreadLocal.withInitial(() -> false); - public final static Map waitingTreads = new ConcurrentHashMap<>(); - public static Map joinRequestedThreads = new ConcurrentHashMap<>(); private long id; private final Object lock; - //private final boolean[] started; - private Thread executingThread; + private final boolean[] started; private Suppliertask; private static long sequence = 0L; @@ -35,20 +29,18 @@ public class ThreadValue extends Value { id = sequence++; lock = new Object(); - executingThread = null; - //started = new boolean[]{false}; + started = new boolean[]{false}; task = () -> { - if (executingThread == null) + if (!started[0]) { - CarpetSettings.LOG.error("Setting the thread"); - executingThread = Thread.currentThread(); + started[0] = true; return function.lazyEval(ctx, Context.NONE, expr, token, Collections.emptyList()).evalValue(ctx); } return null; }; taskFuture = CompletableFuture.supplyAsync( - () -> { synchronized (lock) { CarpetSettings.LOG.error("getting in future"); Value ret = task.get(); CarpetSettings.LOG.error("gotten in future"); return ret;} }, + () -> { synchronized (lock) { return task.get(); } }, executorServices.computeIfAbsent(pool, (v) -> (ThreadPoolExecutor)Executors.newCachedThreadPool()) ); Thread.yield(); @@ -68,45 +60,26 @@ public class ThreadValue extends Value public Value join() { - CarpetSettings.LOG.error("in join "+id); - if (executingThread == null) + if (!started[0]) { - CarpetSettings.LOG.error("not started "+id); - Value res; - waitingTreads.put(Thread.currentThread(), 1); - try + synchronized (lock) { - CarpetSettings.LOG.error(" getting result "+id); - res = task.get(); - CarpetSettings.LOG.error(" result got "+id); + if (!started[0]) + { + Value res = task.get(); + taskFuture.complete(res); + return res; + } } - finally - { - waitingTreads.remove(Thread.currentThread()); - } - taskFuture.complete(res); - return res; } try { - synchronized (waitingTreads) - { - if (waitingTreads.containsKey(executingThread)) - throw new InternalExpressionException("Cannot wait/join on world accessing thread"); - waitingTreads.put(executingThread, 1); - } - CarpetSettings.LOG.error("getting future "+id); return taskFuture.get(); } catch (InterruptedException | ExecutionException e) { return Value.NULL; } - finally - { - waitingTreads.remove(executingThread); - CarpetSettings.LOG.error(" future gotten "+id); - } } public boolean isFinished() @@ -120,7 +93,7 @@ public class ThreadValue extends Value { if (!taskFuture.isDone()) { - executingThread = Thread.currentThread(); + started[0] = true; taskFuture.complete(Value.NULL); } } diff --git a/src/main/resources/carpet.mixins.json b/src/main/resources/carpet.mixins.json index 90b4e437..5ce2fe8d 100644 --- a/src/main/resources/carpet.mixins.json +++ b/src/main/resources/carpet.mixins.json @@ -143,7 +143,6 @@ "FallingBlockEntity_scarpetEventsMixin", "ChunkTicketManager_scarpetMixin", "ServerChunkManager_scarpetMixin", - "ServerChunkManager_scarpetThreadingProtectionMixin", "PistonBlock_movableTEMixin", From 2ff7988251716c7e748e7754fed6b2f706fd558f Mon Sep 17 00:00:00 2001 From: gnembon Date: Tue, 17 Dec 2019 14:41:47 -0400 Subject: [PATCH 3/3] resolved joins from main thread as illegal --- .../java/carpet/script/CarpetExpression.java | 15 +++++++-- src/main/java/carpet/script/Expression.java | 12 +++++-- .../java/carpet/script/value/ThreadValue.java | 33 +++---------------- 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/src/main/java/carpet/script/CarpetExpression.java b/src/main/java/carpet/script/CarpetExpression.java index e5a27ac0..ea198f30 100644 --- a/src/main/java/carpet/script/CarpetExpression.java +++ b/src/main/java/carpet/script/CarpetExpression.java @@ -10,6 +10,7 @@ import carpet.script.exception.CarpetExpressionException; import carpet.script.exception.ExitStatement; import carpet.script.exception.ExpressionException; import carpet.script.exception.InternalExpressionException; +import carpet.CarpetSettings; import carpet.script.value.BlockValue; import carpet.script.value.EntityValue; import carpet.script.value.FunctionValue; @@ -19,8 +20,8 @@ import carpet.script.value.NBTSerializableValue; import carpet.script.value.NullValue; import carpet.script.value.NumericValue; import carpet.script.value.StringValue; +import carpet.script.value.ThreadValue; import carpet.script.value.Value; -import carpet.CarpetSettings; import carpet.utils.BlockInfo; import carpet.utils.Messenger; import com.google.common.collect.Sets; @@ -101,7 +102,6 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.UUID; -import java.util.concurrent.CompletableFuture; import java.util.function.BiFunction; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -3006,6 +3006,17 @@ public class CarpetExpression return (_c, _t) -> res; // pass through for variables }); + //"overidden" native call to cancel if on main thread + this.expr.addLazyFunction("join_task", 1, (c, t, lv) -> { + if (((CarpetContext)c).s.getMinecraftServer().isOnThread()) + throw new InternalExpressionException("'join_task' cannot be called from main thread to avoid deadlocks"); + Value v = lv.get(0).evalValue(c); + if (!(v instanceof ThreadValue)) + throw new InternalExpressionException("'join_task' could only be used with a task value"); + Value ret = ((ThreadValue) v).join(); + return (_c, _t) -> ret; + }); + this.expr.addLazyFunction("logger", 1, (c, t, lv) -> { Value res = lv.get(0).evalValue(c); diff --git a/src/main/java/carpet/script/Expression.java b/src/main/java/carpet/script/Expression.java index 9af5fa2f..facb3504 100644 --- a/src/main/java/carpet/script/Expression.java +++ b/src/main/java/carpet/script/Expression.java @@ -11,7 +11,13 @@ import carpet.script.Fluff.QuadFunction; import carpet.script.Fluff.QuinnFunction; import carpet.script.Fluff.SexFunction; import carpet.script.Fluff.TriFunction; -import carpet.script.exception.*; +import carpet.script.exception.BreakStatement; +import carpet.script.exception.ContinueStatement; +import carpet.script.exception.ExitStatement; +import carpet.script.exception.ExpressionException; +import carpet.script.exception.InternalExpressionException; +import carpet.script.exception.ReturnStatement; +import carpet.script.exception.ThrowStatement; import carpet.script.value.AbstractListValue; import carpet.script.value.ContainerValueInterface; import carpet.script.value.FunctionSignatureValue; @@ -3040,12 +3046,12 @@ public class Expression implements Cloneable return ((ThreadValue) v).join(); }); - addUnaryFunction("stop_task", (v) -> { + /*addUnaryFunction("stop_task", (v) -> { if (!(v instanceof ThreadValue)) throw new InternalExpressionException("'join_task' could only be used with a task value"); ((ThreadValue) v).stop(); return Value.NULL; - }); + });*/ addUnaryFunction("is_task_completed", (v) -> { if (!(v instanceof ThreadValue)) diff --git a/src/main/java/carpet/script/value/ThreadValue.java b/src/main/java/carpet/script/value/ThreadValue.java index 382cd6b2..667e087a 100644 --- a/src/main/java/carpet/script/value/ThreadValue.java +++ b/src/main/java/carpet/script/value/ThreadValue.java @@ -12,36 +12,24 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; import java.util.concurrent.ThreadPoolExecutor; -import java.util.function.Supplier; public class ThreadValue extends Value { private CompletableFuture taskFuture; private long id; private final Object lock; - private final boolean[] started; - private Suppliertask; private static long sequence = 0L; - private static final Map executorServices = new HashMap<>(); public ThreadValue(Value pool, FunctionValue function, Expression expr, Tokenizer.Token token, Context ctx) { id = sequence++; lock = new Object(); - started = new boolean[]{false}; - task = () -> - { - if (!started[0]) - { - started[0] = true; - return function.lazyEval(ctx, Context.NONE, expr, token, Collections.emptyList()).evalValue(ctx); - } - return null; - }; taskFuture = CompletableFuture.supplyAsync( - () -> { synchronized (lock) { return task.get(); } }, - executorServices.computeIfAbsent(pool, (v) -> (ThreadPoolExecutor)Executors.newCachedThreadPool()) + () -> { synchronized (lock) { + return function.lazyEval(ctx, Context.NONE, expr, token, Collections.emptyList()).evalValue(ctx); + } }, + executorServices.computeIfAbsent(pool, (v) -> (ThreadPoolExecutor)Executors.newCachedThreadPool()) ); Thread.yield(); } @@ -60,18 +48,6 @@ public class ThreadValue extends Value public Value join() { - if (!started[0]) - { - synchronized (lock) - { - if (!started[0]) - { - Value res = task.get(); - taskFuture.complete(res); - return res; - } - } - } try { return taskFuture.get(); @@ -93,7 +69,6 @@ public class ThreadValue extends Value { if (!taskFuture.isDone()) { - started[0] = true; taskFuture.complete(Value.NULL); } }