CC: Tweaked

CC: Tweaked

Mods
57M Downloads

Turning multiple new computers/peripherals at once can cause them to be assigned ID 0.

lurr opened this issue ยท 2 comments

commented

Environment:

  • Minecraft 1.12.2
  • CC: Tweaked 1.86.0 (-squid-custom.jar, switchcraft server build)
  • Forge 2847 / SpongeForge 7.1.7-RC3768

Reproduction Steps:

  1. Place a group of new computers of the same network (more is better)
  2. After connecting all to the network, turnOn() all the computers.
peripheral.find("computer", function(name, computer)
    computer.turnOn()
end)
  1. After some attempts, one of the computers will be given ID 0.

Reproduction script (requires command computers)

--// How many computers this script will test with.
local size = 10
 
local pos = { commands.getBlockPosition() }
      pos = { x = pos[1], y = pos[2], z = pos[3] }
 
--// Create a new set of computers for reproduction
for x = pos.x+1, pos.x+size do
    commands.setblock(x, pos.y, pos.z, "minecraft:air")
    commands.setblock(x, pos.y, pos.z, "computercraft:computer")
end
 
--// Create modems underneath computers.
for x = pos.x, pos.x+size do
    commands.setblock(x, pos.y-1, pos.z, "computercraft:wired_modem_full")
end
 
print("Turn on all modems, then press any key.")
os.pullEvent("key")
 
peripheral.find("computer", function (name, comp)
    comp.turnOn()
end)
 
print("The bug should have now been reproduced.")

When the bug occurs, the following error also appears in the console

[10:33:01] [ComputerCraft-Computer-Runner-5/ERROR] [computercraft]: Cannot parse ID file './world/computer/lastid.txt', perhaps it is corrupt?
java.lang.NumberFormatException: null
        at java.lang.Integer.parseInt(Integer.java:542) ~[?:1.8.0_241]
        at java.lang.Integer.parseInt(Integer.java:615) ~[?:1.8.0_241]
        at dan200.computercraft.shared.util.IDAssigner.getNextID(IDAssigner.java:96) [IDAssigner.class:?]
        at dan200.computercraft.shared.util.IDAssigner.getNextIDFromDirectory(IDAssigner.java:31) [IDAssigner.class:?]
        at dan200.computercraft.shared.util.IDAssigner.getNextIDFromDirectory(IDAssigner.java:21) [IDAssigner.class:?]
        at dan200.computercraft.ComputerCraft.createUniqueNumberedSaveDir(ComputerCraft.java:376) [ComputerCraft.class:?]
        at sun.reflect.GeneratedMethodAccessor46.invoke(Unknown Source) ~[?:?]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_241]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_241]
        at dan200.computercraft.api.ComputerCraftAPI.createUniqueNumberedSaveDir(ComputerCraftAPI.java:90) [ComputerCraftAPI.class:?]
        at dan200.computercraft.shared.computer.core.ServerComputer.assignNewID(ServerComputer.java:388) [ServerComputer.class:?]
        at dan200.computercraft.core.computer.Computer.assignID(Computer.java:144) [Computer.class:?]
        at dan200.computercraft.core.computer.ComputerExecutor.getRootMount(ComputerExecutor.java:345) [ComputerExecutor.class:?]
        at dan200.computercraft.core.computer.ComputerExecutor.createFileSystem(ComputerExecutor.java:357) [ComputerExecutor.class:?]
        at dan200.computercraft.core.computer.ComputerExecutor.turnOn(ComputerExecutor.java:431) [ComputerExecutor.class:?]
        at dan200.computercraft.core.computer.ComputerExecutor.work(ComputerExecutor.java:585) [ComputerExecutor.class:?]
        at dan200.computercraft.core.computer.ComputerThread$TaskRunner.run(ComputerThread.java:504) [ComputerThread$TaskRunner.class:?]
        at java.lang.Thread.run(Thread.java:748) [?:1.8.0_241]

This leads me to believe that this is caused by concurrent modification of lastid(_.+)?.txt files, and a lock needs to be created to prevent multiple computers/peripherals from updating the file at once.

commented

Fixed in 1.14?

https://github.com/SquidDev-CC/CC-Tweaked/blob/419f29321adbc1bb5370788cf5e83b23f1827656/src/main/java/dan200/computercraft/shared/util/IDAssigner.java#L60

But yeah, chucking synchronized on there should be enough to fix this. Interesting, I'd somehow got into my head that dan200/ComputerCraft#454 meant that ID assignment also only happened on the main thread, but clearly this isn't true at all.

commented

Sidenote: Some testing with the script above can also lead to duplicate computer IDs.
This can be shown by the small snippet after the turnOn() calls:

--// Wait for all computers to turn on.
sleep(1)

peripheral.find("computer", function (name, comp)
    print(comp.getID())
end)

This produced the followed result:
image