diff --git a/projects/common/src/main/java/dan200/computercraft/shared/turtle/apis/TurtleAPI.java b/projects/common/src/main/java/dan200/computercraft/shared/turtle/apis/TurtleAPI.java index 3d1dd6ac0..ef735d584 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/turtle/apis/TurtleAPI.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/turtle/apis/TurtleAPI.java @@ -219,7 +219,7 @@ public class TurtleAPI implements ILuaAPI { * @cc.since 1.4 */ @LuaFunction - public final MethodResult place(IArguments args) { + public final MethodResult place(IArguments args) throws LuaException { return trackCommand(new TurtlePlaceCommand(InteractDirection.FORWARD, args.getAll())); } @@ -235,7 +235,7 @@ public class TurtleAPI implements ILuaAPI { * @see #place For more information about placing items. */ @LuaFunction - public final MethodResult placeUp(IArguments args) { + public final MethodResult placeUp(IArguments args) throws LuaException { return trackCommand(new TurtlePlaceCommand(InteractDirection.UP, args.getAll())); } @@ -251,7 +251,7 @@ public class TurtleAPI implements ILuaAPI { * @see #place For more information about placing items. */ @LuaFunction - public final MethodResult placeDown(IArguments args) { + public final MethodResult placeDown(IArguments args) throws LuaException { return trackCommand(new TurtlePlaceCommand(InteractDirection.DOWN, args.getAll())); } diff --git a/projects/core-api/src/main/java/dan200/computercraft/api/lua/IArguments.java b/projects/core-api/src/main/java/dan200/computercraft/api/lua/IArguments.java index 9bfffd9ea..f984ad35e 100644 --- a/projects/core-api/src/main/java/dan200/computercraft/api/lua/IArguments.java +++ b/projects/core-api/src/main/java/dan200/computercraft/api/lua/IArguments.java @@ -35,9 +35,15 @@ public interface IArguments { * * @param index The argument number. * @return The argument's value, or {@code null} if not present. + * @throws LuaException If the argument cannot be converted to Java. This should be thrown in extraneous + * circumstances (if the conversion would allocate too much memory) and should + * not be thrown if the original argument is not present or is an unsupported + * data type (such as a function or userdata). + * @throws IllegalStateException If accessing these arguments outside the scope of the original function. See + * {@link #escapes()}. */ @Nullable - Object get(int index); + Object get(int index) throws LuaException; /** * Get the type name of the argument at the specific index. @@ -49,9 +55,7 @@ public interface IArguments { * @return The name of this type. * @see LuaValues#getType(Object) */ - default String getType(int index) { - return LuaValues.getType(get(index)); - } + String getType(int index); /** * Drop a number of arguments. The returned arguments instance will access arguments at position {@code i + count}, @@ -62,7 +66,14 @@ public interface IArguments { */ IArguments drop(int count); - default Object[] getAll() { + /** + * Get an array containing all as {@link Object}s. + * + * @return All arguments. + * @throws LuaException If an error occurred while fetching an argument. + * @see #get(int) To get a single argument. + */ + default Object[] getAll() throws LuaException { var result = new Object[count()]; for (var i = 0; i < result.length; i++) result[i] = get(i); return result; @@ -421,4 +432,26 @@ public interface IArguments { default Map optTable(int index, @Nullable Map def) throws LuaException { return optTable(index).orElse(def); } + + /** + * Create a version of these arguments which escapes the scope of the current function call. + *

+ * Some {@link IArguments} implementations provide a view over the underlying Lua data structures, allowing for + * zero-copy implementations of some methods (such as {@link #getTableUnsafe(int)} or {@link #getBytes(int)}). + * However, this means the arguments can only be accessed inside the current function call. + *

+ * If the arguments escape the scope of the current call (for instance, are later accessed on the main server + * thread), then these arguments must be marked as "escaping", which may choose to perform a copy of the underlying + * arguments. + *

+ * If you are using {@link LuaFunction#mainThread()}, this will be done automatically. However, if you call + * {@link ILuaContext#issueMainThreadTask(LuaTask)} (or similar), then you will need to mark arguments as escaping + * yourself. + * + * @return An {@link IArguments} instance which can escape the current scope. May be {@code this}. + * @throws LuaException For the same reasons as {@link #get(int)}. + */ + default IArguments escapes() throws LuaException { + return this; + } } diff --git a/projects/core-api/src/main/java/dan200/computercraft/api/lua/ObjectArguments.java b/projects/core-api/src/main/java/dan200/computercraft/api/lua/ObjectArguments.java index fd3ee1019..14e36fb44 100644 --- a/projects/core-api/src/main/java/dan200/computercraft/api/lua/ObjectArguments.java +++ b/projects/core-api/src/main/java/dan200/computercraft/api/lua/ObjectArguments.java @@ -51,6 +51,11 @@ public final class ObjectArguments implements IArguments { return index >= args.size() ? null : args.get(index); } + @Override + public String getType(int index) { + return LuaValues.getType(get(index)); + } + @Override public Object[] getAll() { return args.toArray(); diff --git a/projects/core/src/main/java/dan200/computercraft/core/apis/OSAPI.java b/projects/core/src/main/java/dan200/computercraft/core/apis/OSAPI.java index 38ad39759..0d2e2347f 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/apis/OSAPI.java +++ b/projects/core/src/main/java/dan200/computercraft/core/apis/OSAPI.java @@ -136,7 +136,7 @@ public class OSAPI implements ILuaAPI { * @cc.see os.pullEvent To pull the event queued */ @LuaFunction - public final void queueEvent(String name, IArguments args) { + public final void queueEvent(String name, IArguments args) throws LuaException { apiEnvironment.queueEvent(name, args.drop(1).getAll()); } @@ -165,8 +165,8 @@ public class OSAPI implements ILuaAPI { * timer from firing. * * @param token The ID of the timer to cancel. - * @see #startTimer To start a timer. * @cc.since 1.6 + * @see #startTimer To start a timer. */ @LuaFunction public final void cancelTimer(int token) { @@ -351,7 +351,7 @@ public class OSAPI implements ILuaAPI { * January 1970 in the UTC timezone. * * If called with {@code local}, returns the number of milliseconds since 1 * January 1970 in the server's local timezone. - * + *

* :::info * The {@code ingame} time zone assumes that one Minecraft day consists of 86,400,000 * milliseconds. Since one in-game day is much faster than a real day (20 minutes), this diff --git a/projects/core/src/main/java/dan200/computercraft/core/asm/LuaMethod.java b/projects/core/src/main/java/dan200/computercraft/core/asm/LuaMethod.java index f9742de15..7de22ff1b 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/asm/LuaMethod.java +++ b/projects/core/src/main/java/dan200/computercraft/core/asm/LuaMethod.java @@ -10,7 +10,7 @@ import java.util.Collections; public interface LuaMethod { Generator GENERATOR = new Generator<>(LuaMethod.class, Collections.singletonList(ILuaContext.class), - m -> (target, context, args) -> context.executeMainThreadTask(() -> ResultHelpers.checkNormalResult(m.apply(target, context, args))) + m -> (target, context, args) -> context.executeMainThreadTask(() -> ResultHelpers.checkNormalResult(m.apply(target, context, args.escapes()))) ); IntCache DYNAMIC = new IntCache<>( diff --git a/projects/core/src/main/java/dan200/computercraft/core/asm/PeripheralMethod.java b/projects/core/src/main/java/dan200/computercraft/core/asm/PeripheralMethod.java index 2096ebf8d..f20b85998 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/asm/PeripheralMethod.java +++ b/projects/core/src/main/java/dan200/computercraft/core/asm/PeripheralMethod.java @@ -15,7 +15,7 @@ import java.util.Arrays; public interface PeripheralMethod { Generator GENERATOR = new Generator<>(PeripheralMethod.class, Arrays.asList(ILuaContext.class, IComputerAccess.class), - m -> (target, context, computer, args) -> context.executeMainThreadTask(() -> ResultHelpers.checkNormalResult(m.apply(target, context, computer, args))) + m -> (target, context, computer, args) -> context.executeMainThreadTask(() -> ResultHelpers.checkNormalResult(m.apply(target, context, computer, args.escapes()))) ); IntCache DYNAMIC = new IntCache<>( diff --git a/projects/core/src/main/java/dan200/computercraft/core/lua/TableImpl.java b/projects/core/src/main/java/dan200/computercraft/core/lua/TableImpl.java index 68fe755dc..0ffa65d76 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/lua/TableImpl.java +++ b/projects/core/src/main/java/dan200/computercraft/core/lua/TableImpl.java @@ -103,7 +103,7 @@ class TableImpl implements dan200.computercraft.api.lua.LuaTable } private void checkValid() { - if (arguments.closed) { + if (arguments.isClosed()) { throw new IllegalStateException("Cannot use LuaTable after IArguments has been released"); } } diff --git a/projects/core/src/main/java/dan200/computercraft/core/lua/VarargArguments.java b/projects/core/src/main/java/dan200/computercraft/core/lua/VarargArguments.java index d0abf9676..5f60e1abd 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/lua/VarargArguments.java +++ b/projects/core/src/main/java/dan200/computercraft/core/lua/VarargArguments.java @@ -7,6 +7,8 @@ package dan200.computercraft.core.lua; import dan200.computercraft.api.lua.IArguments; import dan200.computercraft.api.lua.LuaException; import dan200.computercraft.api.lua.LuaValues; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.squiddev.cobalt.*; import javax.annotation.Nullable; @@ -16,20 +18,64 @@ import java.util.Optional; import static org.squiddev.cobalt.Constants.NAME; final class VarargArguments implements IArguments { - private static final VarargArguments EMPTY = new VarargArguments(Constants.NONE); + private static final Logger LOG = LoggerFactory.getLogger(VarargArguments.class); + + private static final VarargArguments EMPTY = new VarargArguments(Constants.NONE); + private static boolean reportedIllegalGet; + + static { + EMPTY.escapes = EMPTY.closed = true; + } - boolean closed; private final Varargs varargs; - private @Nullable Object[] cache; + + private volatile boolean closed; + private final VarargArguments root; + + private @Nullable ArraySlice cache; + private @Nullable ArraySlice typeNames; + + private boolean escapes; private VarargArguments(Varargs varargs) { this.varargs = varargs; + root = this; + } + + private VarargArguments(Varargs varargs, VarargArguments root, int offset) { + this.varargs = varargs; + this.root = root; + escapes = root.escapes; + cache = root.cache == null ? null : root.cache.drop(offset); + typeNames = root.typeNames == null ? null : root.typeNames.drop(offset); } static VarargArguments of(Varargs values) { return values == Constants.NONE ? EMPTY : new VarargArguments(values); } + boolean isClosed() { + return root.closed; + } + + private void checkAccessible() { + if (isClosed() && !escapes) throwInaccessible(); + } + + private void throwInaccessible() { + var error = new IllegalStateException("Function arguments have escaped their original scope."); + if (!reportedIllegalGet) { + reportedIllegalGet = true; + LOG.error( + "A function attempted to access arguments outside the scope of the original function. This is probably " + + "caused by the function scheduling work on the main thread. You may need to call IArguments.escapes().", + error + ); + } + + throw error; + } + @Override public int count() { return varargs.count(); @@ -38,26 +84,37 @@ final class VarargArguments implements IArguments { @Nullable @Override public Object get(int index) { + checkAccessible(); if (index < 0 || index >= varargs.count()) return null; var cache = this.cache; if (cache == null) { - cache = this.cache = new Object[varargs.count()]; + cache = this.cache = new ArraySlice<>(new Object[varargs.count()], 0); } else { - var existing = cache[index]; + var existing = cache.get(index); if (existing != null) return existing; } - return cache[index] = CobaltLuaMachine.toObject(varargs.arg(index + 1), null); + var arg = varargs.arg(index + 1); + + // This holds as either a) the arguments are not closed or b) the arguments were escaped, in which case + // tables should have been converted already. + assert !isClosed() || !(arg instanceof LuaTable) : "Converting a LuaTable after arguments were closed."; + + var converted = CobaltLuaMachine.toObject(arg, null); + cache.set(index, converted); + return converted; } @Override public String getType(int index) { + checkAccessible(); + var value = varargs.arg(index + 1); - if (value instanceof LuaTable || value instanceof LuaUserdata) { - var metatable = value.getMetatable(null); - if (metatable != null && metatable.rawget(NAME) instanceof LuaString s) return s.toString(); - } + + // If we've escaped, read it from the precomputed list, otherwise get the custom name. + var name = escapes ? (typeNames == null ? null : typeNames.get(index)) : getCustomType(value); + if (name != null) return name; return value.typeName(); } @@ -66,11 +123,15 @@ final class VarargArguments implements IArguments { public IArguments drop(int count) { if (count < 0) throw new IllegalStateException("count cannot be negative"); if (count == 0) return this; - return new VarargArguments(varargs.subargs(count + 1)); + + var newArgs = varargs.subargs(count + 1); + if (newArgs == Constants.NONE) return EMPTY; + return new VarargArguments(newArgs, this, count); } @Override public double getDouble(int index) throws LuaException { + checkAccessible(); var value = varargs.arg(index + 1); if (!(value instanceof LuaNumber)) throw LuaValues.badArgument(index, "number", value.typeName()); return value.toDouble(); @@ -78,6 +139,7 @@ final class VarargArguments implements IArguments { @Override public long getLong(int index) throws LuaException { + checkAccessible(); var value = varargs.arg(index + 1); if (!(value instanceof LuaNumber)) throw LuaValues.badArgument(index, "number", value.typeName()); return value instanceof LuaInteger ? value.toInteger() : (long) LuaValues.checkFinite(index, value.toDouble()); @@ -85,6 +147,7 @@ final class VarargArguments implements IArguments { @Override public ByteBuffer getBytes(int index) throws LuaException { + checkAccessible(); var value = varargs.arg(index + 1); if (!(value instanceof LuaString str)) throw LuaValues.badArgument(index, "string", value.typeName()); return str.toBuffer(); @@ -92,6 +155,7 @@ final class VarargArguments implements IArguments { @Override public Optional optBytes(int index) throws LuaException { + checkAccessible(); var value = varargs.arg(index + 1); if (value.isNil()) return Optional.empty(); if (!(value instanceof LuaString str)) throw LuaValues.badArgument(index, "string", value.typeName()); @@ -100,9 +164,7 @@ final class VarargArguments implements IArguments { @Override public dan200.computercraft.api.lua.LuaTable getTableUnsafe(int index) throws LuaException { - if (closed) { - throw new IllegalStateException("Cannot use getTableUnsafe after IArguments has been closed."); - } + if (isClosed()) throw new IllegalStateException("Cannot use getTableUnsafe after IArguments has been closed."); var value = varargs.arg(index + 1); if (!(value instanceof LuaTable)) throw LuaValues.badArgument(index, "table", value.typeName()); @@ -111,9 +173,7 @@ final class VarargArguments implements IArguments { @Override public Optional> optTableUnsafe(int index) throws LuaException { - if (closed) { - throw new IllegalStateException("Cannot use optTableUnsafe after IArguments has been closed."); - } + if (isClosed()) throw new IllegalStateException("Cannot use optTableUnsafe after IArguments has been closed."); var value = varargs.arg(index + 1); if (value.isNil()) return Optional.empty(); @@ -121,7 +181,61 @@ final class VarargArguments implements IArguments { return Optional.of(new TableImpl(this, (LuaTable) value)); } - public void close() { + @Override + public IArguments escapes() { + if (escapes) return this; + + var cache = this.cache; + var typeNames = this.typeNames; + + for (int i = 0, count = varargs.count(); i < count; i++) { + var arg = varargs.arg(i + 1); + + // Convert tables. + if (arg instanceof LuaTable) { + if (cache == null) cache = new ArraySlice<>(new Object[count], 0); + cache.set(i, CobaltLuaMachine.toObject(arg, null)); + } + + // Fetch custom type names. + var typeName = getCustomType(arg); + if (typeName != null) { + if (typeNames == null) typeNames = new ArraySlice<>(new String[count], 0); + typeNames.set(i, typeName); + } + } + + escapes = true; + this.cache = cache; + this.typeNames = typeNames; + return this; + } + + void close() { closed = true; } + + private static @Nullable String getCustomType(LuaValue arg) { + if (!(arg instanceof LuaTable) && !(arg instanceof LuaUserdata)) return null; + + var metatable = arg.getMetatable(null); + return metatable != null && metatable.rawget(NAME) instanceof LuaString s ? s.toString() : null; + } + + private record ArraySlice(T[] array, int offset) { + // FIXME: We should be able to remove the @Nullables if we update NullAway. + + @Nullable + T get(int index) { + return array[offset + index]; + } + + void set(int index, @Nullable T value) { + array[offset + index] = value; + } + + ArraySlice drop(int count) { + return new ArraySlice<>(array, offset + count); + } + } } diff --git a/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java b/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java index 19c70be75..afdc54afa 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java +++ b/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java @@ -118,7 +118,7 @@ public class ComputerBootstrap { } @LuaFunction - public final void log(IArguments arguments) { + public final void log(IArguments arguments) throws LuaException { LOG.info("[Computer] {}", Arrays.toString(arguments.getAll())); } diff --git a/projects/core/src/test/java/dan200/computercraft/core/lua/VarargArgumentsTest.java b/projects/core/src/test/java/dan200/computercraft/core/lua/VarargArgumentsTest.java new file mode 100644 index 000000000..7a76ea562 --- /dev/null +++ b/projects/core/src/test/java/dan200/computercraft/core/lua/VarargArgumentsTest.java @@ -0,0 +1,63 @@ +// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers +// +// SPDX-License-Identifier: MPL-2.0 + +package dan200.computercraft.core.lua; + +import dan200.computercraft.api.lua.LuaException; +import org.junit.jupiter.api.Test; +import org.squiddev.cobalt.Constants; +import org.squiddev.cobalt.LuaTable; +import org.squiddev.cobalt.ValueFactory; + +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class VarargArgumentsTest { + private static LuaTable tableWithCustomType() { + var metatable = new LuaTable(); + metatable.rawset(Constants.NAME, ValueFactory.valueOf("some type")); + + var table = new LuaTable(); + table.setMetatable(null, metatable); + return table; + } + + @Test + public void testGet() { + var args = VarargArguments.of(tableWithCustomType()); + assertEquals(Map.of(), args.get(0)); + assertEquals("some type", args.getType(0)); + } + + @Test + public void testGetAfterEscape() { + var args = VarargArguments.of(tableWithCustomType()); + args.escapes(); + args.close(); + + assertEquals(Map.of(), args.get(0)); + assertEquals("some type", args.getType(0)); + } + + @Test + public void testGetAfterEscapeDrop() throws LuaException { + var args = VarargArguments.of(ValueFactory.varargsOf(Constants.NIL, tableWithCustomType())); + args.escapes(); + args.close(); + + assertEquals(Map.of(), args.drop(1).get(0)); + assertEquals("some type", args.drop(1).getType(0)); + } + + @Test + public void testGetAfterClose() { + var args = VarargArguments.of(tableWithCustomType()); + args.close(); + + assertThrows(IllegalStateException.class, () -> args.get(0)); + assertThrows(IllegalStateException.class, () -> args.getType(0)); + } +}