cc-tweaked / CC-Tweaked

Just another ComputerCraft fork
https://tweaked.cc
883 stars 207 forks source link

[Issue help request] Peripheral take too long to connect back (?) #1536

Closed SirEdvin closed 11 months ago

SirEdvin commented 11 months ago

So, I have this strange issue. When program continuously moving item between barrels and someone open one of them, is is possible that program will break with "dan200.computercraft.api.peripheral.NotAttachedException: You are not attached to this computer" exception inside wired modem (?) , which I believe is intended behavior. However, it seems that something in UPW causes this bug to occurs, because it seems it don't occur with just CC:R/CC:T.

I am not sure, from what I even can start with this issue, because I though that since everything happens in main thread, delay between attaching/reattaching peripheral should be nearly zero for computers. Maybe anyone have a suggestions, because I am out of ideas.

Minimal reproducing steps:

2023-07-25_12 16 17

Information about attaching/detaching seems fine: зображення

Linguardium commented 11 months ago
RemotePeripheralWrapper 1909671818 attach on Thread Server thread
PluggedPeripheral attach on Thread Server thread
callMethod from ComputerCraft-Coroutine-0
RemotePeripheralWrapper 1909671818 detach on ThreadServer thread
BoundMethod apply from ComputerCraft-Coroutine-0
PluggedPeripheral detach from Thread Server thread
RemotePeripheralWrapper 1864834915 attach on Thread Server thread
PluggedPeripheral attach on Thread Server thread
pushItems called from minecraft:barrel_0$$1909671818 - attached: false

It appears that because dynamic calls happen on the lua thread as indicated in the javadocs, they must be thread safe, but you are passing it an IComputerAccess that can be invalidated (detached) on the server thread during block updates/TickManager.tick prior to the queued main thread task execution.

The invalidation issue seems to be caused by your provider generating new peripherals every time there is a block update, and the PluggablePeripheral equals check only determining if the objects are the same object (which they will never be)

Having not dug into the code further, my recommendation would be to add a methods and plugins check to your PluggablePeripheral class's equals check

SquidDev commented 11 months ago

@Linguardium has hit the nail on the head. The original peripheral gets detached, and so the main-thread task fails. This has been a long standing issue - #893 discusses my ideas to fix this - but normally it's pretty rare. In this case, the problem is exacerbated because the peripheral keeps being invalidated - fixing that should mostly resolve this.

SirEdvin commented 11 months ago

Having not dug into the code further, my recommendation would be to add a methods and plugins check to your PluggablePeripheral class's equals check

From comparing GenericPeripheral with my PluggablePeripheral, it seems only gap that I can found. I will try to fix this shortly (probably till the end of the week).

I really want to thanks for this investigation, this is really cool!

SirEdvin commented 11 months ago

So, this is resolved, thanks for helping me with this!