From 474f571798722e5964a0af2eb8805ab117295835 Mon Sep 17 00:00:00 2001 From: SquidDev Date: Mon, 25 Feb 2019 19:11:35 +0000 Subject: [PATCH] Attach peripherals directly rather than deferring Prior to this change we would schedule a new task which attached peripherals on the ComputerThread on the empty task queue. This had a couple of issues: - Slow running tasks on the computer thread could result in delays in peripherals being attached (technically, though rarely seen in practice). - Now that the ComputerThread runs tasks at once, there was a race condition in computers being turned on/off and peripherals being attached/detached. Note, while the documentation said that peripherals would only be (at|de)tached on the computer thread, wired modems would attach on the server thread, so this was not the case in practice. One should be aware that peripherals are still detached on the computer thread, most notably when turning a computer on/off. This is almost definitely going to break some less well-behaved mods, and possible some of the well behaved ones. I've tested this on SC, so it definitely works fine with Computronics and Plethora. --- .../api/peripheral/IPeripheral.java | 45 +++++++------- .../core/apis/PeripheralAPI.java | 60 ++----------------- 2 files changed, 29 insertions(+), 76 deletions(-) diff --git a/src/main/java/dan200/computercraft/api/peripheral/IPeripheral.java b/src/main/java/dan200/computercraft/api/peripheral/IPeripheral.java index b7c072f06..00edad3a6 100644 --- a/src/main/java/dan200/computercraft/api/peripheral/IPeripheral.java +++ b/src/main/java/dan200/computercraft/api/peripheral/IPeripheral.java @@ -41,8 +41,8 @@ public interface IPeripheral * This is called when a lua program on an attached computer calls {@code peripheral.call()} with * one of the methods exposed by {@link #getMethodNames()}. * - * Be aware that this will be called from the ComputerCraft Lua thread, and must be thread-safe - * when interacting with Minecraft objects. + * Be aware that this will be called from the ComputerCraft Lua thread, and must be thread-safe when interacting + * with Minecraft objects. * * @param computer The interface to the computer that is making the call. Remember that multiple * computers can be attached to a peripheral at once. @@ -75,20 +75,21 @@ public interface IPeripheral Object[] callMethod( @Nonnull IComputerAccess computer, @Nonnull ILuaContext context, int method, @Nonnull Object[] arguments ) throws LuaException, InterruptedException; /** - * Is called when canAttachToSide has returned true, and a computer is attaching to the peripheral. + * Is called when when a computer is attaching to the peripheral. * * This will occur when a peripheral is placed next to an active computer, when a computer is turned on next to a - * peripheral, or when a turtle travels into a square next to a peripheral. + * peripheral, when a turtle travels into a square next to a peripheral, or when a wired modem adjacent to this + * peripheral is does any of the above. * - * Between calls to attach() and detach(), the attached computer can make method calls on the peripheral using - * {@code peripheral.call()}. This method can be used to keep track of which computers are attached to the - * peripheral, or to take action when attachment occurs. + * Between calls to {@link #attach} and {@link #detach}, the attached computer can make method calls on the + * peripheral using {@code peripheral.call()}. This method can be used to keep track of which computers are attached + * to the peripheral, or to take action when attachment occurs. * - * Be aware that this will be called from the ComputerCraft Lua thread, and must be thread-safe - * when interacting with Minecraft objects. + * Be aware that will be called from both the server thread and ComputerCraft Lua thread, and so must be thread-safe + * and reentrant. * - * @param computer The interface to the computer that is being attached. Remember that multiple - * computers can be attached to a peripheral at once. + * @param computer The interface to the computer that is being attached. Remember that multiple computers can be + * attached to a peripheral at once. * @see #detach */ default void attach( @Nonnull IComputerAccess computer ) @@ -96,19 +97,21 @@ public interface IPeripheral } /** - * Is called when a computer is detaching from the peripheral. + * Called when a computer is detaching from the peripheral. * - * This will occur when a computer shuts down, when the peripheral is removed while attached to computers, - * or when a turtle moves away from a square attached to a peripheral. This method can be used to keep track of - * which computers are attached to the peripheral, or to take action when detachment - * occurs. + * This will occur when a computer shuts down, when the peripheral is removed while attached to computers, when a + * turtle moves away from a block attached to a peripheral, or when a wired modem adjacent to this peripheral is + * detached. * - * Be aware that this will be called from the ComputerCraft Lua thread, and must be thread-safe - * when interacting with Minecraft objects. + * This method can be used to keep track of which computers are attached to the peripheral, or to take action when + * detachment occurs. * - * @param computer The interface to the computer that is being detached. Remember that multiple - * computers can be attached to a peripheral at once. - * @see #detach + * Be aware that this will be called from both the server and ComputerCraft Lua thread, and must be thread-safe + * and reentrant. + * + * @param computer The interface to the computer that is being detached. Remember that multiple computers can be + * attached to a peripheral at once. + * @see #attach */ default void detach( @Nonnull IComputerAccess computer ) { diff --git a/src/main/java/dan200/computercraft/core/apis/PeripheralAPI.java b/src/main/java/dan200/computercraft/core/apis/PeripheralAPI.java index df124e660..41321a067 100644 --- a/src/main/java/dan200/computercraft/core/apis/PeripheralAPI.java +++ b/src/main/java/dan200/computercraft/core/apis/PeripheralAPI.java @@ -12,9 +12,6 @@ import dan200.computercraft.api.lua.ILuaAPI; import dan200.computercraft.api.lua.ILuaContext; import dan200.computercraft.api.lua.LuaException; import dan200.computercraft.api.peripheral.IPeripheral; -import dan200.computercraft.core.computer.Computer; -import dan200.computercraft.core.computer.ComputerThread; -import dan200.computercraft.core.computer.ITask; import dan200.computercraft.core.tracking.TrackingField; import javax.annotation.Nonnull; @@ -256,65 +253,21 @@ public class PeripheralAPI implements ILuaAPI, IAPIEnvironment.IPeripheralChange { // Queue a detachment final PeripheralWrapper wrapper = m_peripherals[side]; - ComputerThread.queueTask( new ITask() - { - @Override - public Computer getOwner() - { - return m_environment.getComputer(); - } - - @Override - public void execute() - { - synchronized( m_peripherals ) - { - if( wrapper.isAttached() ) - { - wrapper.detach(); - } - } - } - }, null ); + if( wrapper.isAttached() ) wrapper.detach(); // Queue a detachment event m_environment.queueEvent( "peripheral_detach", new Object[] { IAPIEnvironment.SIDE_NAMES[side] } ); } // Assign the new peripheral - if( newPeripheral != null ) - { - m_peripherals[side] = new PeripheralWrapper( newPeripheral, IAPIEnvironment.SIDE_NAMES[side] ); - } - else - { - m_peripherals[side] = null; - } + m_peripherals[side] = newPeripheral == null ? null + : new PeripheralWrapper( newPeripheral, IAPIEnvironment.SIDE_NAMES[side] ); if( m_peripherals[side] != null ) { // Queue an attachment final PeripheralWrapper wrapper = m_peripherals[side]; - ComputerThread.queueTask( new ITask() - { - @Override - public Computer getOwner() - { - return m_environment.getComputer(); - } - - @Override - public void execute() - { - synchronized( m_peripherals ) - { - if( m_running && !wrapper.isAttached() ) - { - wrapper.attach(); - } - } - } - }, null ); + if( m_running && !wrapper.isAttached() ) wrapper.attach(); // Queue an attachment event m_environment.queueEvent( "peripheral", new Object[] { IAPIEnvironment.SIDE_NAMES[side] } ); @@ -341,10 +294,7 @@ public class PeripheralAPI implements ILuaAPI, IAPIEnvironment.IPeripheralChange for( int i = 0; i < 6; i++ ) { PeripheralWrapper wrapper = m_peripherals[i]; - if( wrapper != null && !wrapper.isAttached() ) - { - wrapper.attach(); - } + if( wrapper != null && !wrapper.isAttached() ) wrapper.attach(); } } }