Baritone AI pathfinder

Baritone AI pathfinder

72.7k Downloads

ArrayIndexOutOfBoundsException in FasterWorldScanner

ZacSharp opened this issue ยท 13 comments

commented

Some information

Operating system: Linux 5.15.0-47-generic
Java version: 1.8.0_362
Minecraft version: 1.12.2
Baritone version: originally v1.2.17 branched from ff17320 (commit is ZacSharp@03955d9 in case I push it). Reproduced on ff17320 with changes specifically to trigger this.
Other mods (if used): None

Exception, error or logs

original log
reproduction log

How to reproduce

Method 1 (How I found this, did not work again)

  1. Run a random combination of commands which involve placing/breaking blocks
  2. Run #farm

Method 2 (How it happened again)

Apply

diff --git a/src/main/java/baritone/cache/FasterWorldScanner.java b/src/main/java/baritone/cache/FasterWorldScanner.java
index fbdc238ce..6f94981e4 100644
--- a/src/main/java/baritone/cache/FasterWorldScanner.java
+++ b/src/main/java/baritone/cache/FasterWorldScanner.java
@@ -192,6 +192,9 @@ private void visitSection(BlockOptionalMetaLookup lookup, ExtendedBlockStorage s
         if (isInFilter.length == 0) {
             return;
         }
+        try {
+            Thread.sleep(1000);
+        } catch (InterruptedException ignored) {}
 
         BitArray array = ((IBlockStateContainer) section.getData()).getStorage();
         long[] longArray = array.getBackingLongArray();

Then generate a world using the void preset, place some farmland, plant some wheat, give yourself some bonemeal and run #farm.
With this patch applied it also worked in my original test world, but only after manually moving the player around a bit and thus changing the block Baritone bonemeals, leaving the previous one in a partially grown state.

My guess why it happens

Since inFilter had the wrong size and contructing it is separate from extracting the palette array and scanning happens off thread I suspect this happens when the palette is resized between the line directly above the if statement and the line directly after it. This is consistent with a Thread.sleep between them making the exception far easier to encounter and also fits nicely with the fact that adding some partially grown crops helped triggering the exception. I haven't printed out the numbers yet, though I expect to see the palette size crossing a power of two every time AIOOBE is thrown (will test this tomorrow).

Modified settings

None (for method 2)

Final checklist

  • I know how to properly use check boxes
  • I have included the version of Minecraft I'm running, baritone's version and forge mods (if used).
  • I have included logs, exceptions and / or steps to reproduce the issue.
  • I have not added any OwO's or UwU's to this issue.
commented

Actually, I just had this happen when trying to reproduce #3918 on ff17320.
That means no extra delay and no blocks being mined or broken while scanning.

2023-04-22-5.log

commented

I confirmed that when AIOOBE is thrown the palette size has increased (by any amount) between extracting the indices and re-extracting them. It seems like the palette size does not decrease unless the chunk is reloaded, so this can only happen a limited amount of times per chunk load.
latest.log
changes.diff.txt (apply to ff17320)

commented

between extracting
and re-extracting

could we change the order or something then to fix, or make it extract once only somehow...
would try, catch, retry be more performant than properly fixing

commented

3 is easiest
2 is possible with a custom mixin plugin, idk, just changing the flags to make it synchronized probably wont break other mods but it would also make things slower, probably in unintended ways and would be bad with the readwrite lock in sodium
1 is also possible. I rewrote the worldscanner because it was nuking fps anyway, so... putting it on main should only cause a small hiccup. Imo this may be most preferable

commented

Sorry if I was unclear about that: the re-extraction is my debug code to check whether things changed. Basically I read everything a second time after things went wrong.

We could surely try to reduce the probability of writes between palette and storage array access, but it possibility of partially written data would remain. To fix this we could

  1. Make scanning "synchronous" (i.e. block main/use it for scanning, not scan only on main) maybe scanning is fast enough to not cause stutter.
  2. Synchronize access to chunk data. Iirc we're accessing private data here so making the accessor methods synchronized and manually synchronizing ourselves should do the trick. Would need a custom asm transfomer.
  3. Put everything inside a try-catch, maybe allow it to retry once and accept the possibility of weird behavior.

 1. is the clear "once and for all" solution but might turn out unacceptable and 2. in my opinion isn't worth the effort. Some rare race condition doesn't justify rolling a core mod. If we really end up with 3. we can maybe try to do some reordering and additional checks to catch some cases, but I wouldn't expect anything from that. It's not like we can rely on seeing changes made by main, we just have to deal with them if we happen to see (part of) them.

commented

โ€‹1. Seems to be acceptable.

2023-05-07.01-54-09.mp4

Keep it a setting and put in the try-catch anyway (I'd prefer that) or assume it's going to work fine for everyone?

commented

Yeah, setting when async give it 3 tries with try-catch before actually throwing (or maybe settable tries) would be fine

commented

Ran into this a lot when testing for #3726 and got a <sarcasm>tiny bit</sarcasm> distracted. Apparently from some reason a blockstate can have two palette ids making the palette returned by getPalette miss some entries so isInFilter ends up being too short. No idea why it happens, the Minecraft code looks like this should be impossible.

Also FasterWorldScanner really isn't the best at being thread safe. The catch-all at the top level is used a fair bit if you respawn or switch worlds or close the game at the wrong time.

commented

Seems like some Minecraft versions do indeed create duplicate palette entries: https://bugs.mojang.com/browse/MC-237925 / https://bugs.mojang.com/browse/MC-238056.
I'll just make our extraction method keep potential duplicates and waste some more time trying to figure out why duplicates happen in the first place.

commented

I'm seeing this in the log:

[20:58:49] [pool-8-thread-1/INFO]: [STDOUT]: Loaded region successfully in 386ms
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: java.lang.ArrayIndexOutOfBoundsException: Index 25 out of bounds for length 24
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at baritone.utils.BlockStateInterface.getFromChunk(BlockStateInterface.java:174)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at baritone.cache.ChunkPacker.getPathingBlockType(ChunkPacker.java:128)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at baritone.cache.ChunkPacker.pack(ChunkPacker.java:80)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at baritone.cache.CachedWorld$PackerThread.run(CachedWorld.java:312)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at java.base/java.lang.Thread.run(Thread.java:833)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: java.lang.ArrayIndexOutOfBoundsException
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: java.lang.ArrayIndexOutOfBoundsException
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: java.lang.ArrayIndexOutOfBoundsException
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: java.lang.ArrayIndexOutOfBoundsException: Index 24 out of bounds for length 24
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at baritone.utils.BlockStateInterface.getFromChunk(BlockStateInterface.java:174)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at baritone.cache.ChunkPacker.getPathingBlockType(ChunkPacker.java:128)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at baritone.cache.ChunkPacker.pack(ChunkPacker.java:80)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at baritone.cache.CachedWorld$PackerThread.run(CachedWorld.java:312)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: 	at java.base/java.lang.Thread.run(Thread.java:833)
[20:58:50] [pool-8-thread-1/INFO]: [STDERR]: java.lang.ArrayIndexOutOfBoundsException

Is this the related bug? I'm getting it in v1.10.1

commented

Yes, that's probably this bug, though iirc the mc bugtracker claims that this shouldn't happen on newer versions.
I have a fix, but like one or two more prs I had in preparation before I went on vacation it was written on master (1.12.2) and master not being supported anymore when I came back kind of threw me off. Currently I'm distracted by some fascinating new things to learn and try but once I get back to working on Baritone the port won't take long (I wouldn't mind someone else fixing it in the meantime).

commented

same thing with #mine and only in overworld

commented

Can you please upload the stacktrace and error message (or the whole log file)? Also can you please try whether it has to do with your y position (y>=0 vs y<0) rather than nether vs overworld? (that would be #4112)