t2pellet / strawgolem

Adds a cute straw golem to farm for you!
https://www.curseforge.com/minecraft/mc-mods/straw-golem-reborn
GNU Affero General Public License v3.0
16 stars 17 forks source link

`StackOverflowError` in `OctTree` on `insert()` #139

Closed solonovamax closed 5 months ago

solonovamax commented 1 year ago

Hi, I am currently reproducing able to reproduce #120, using version 1.19.2-2.0.1-alpha. As-per this comment, I am opening a new issue.

The issue occurs during runtime, because the straw golem is not in spawn chunks. So, when I load up the world, if I travel into chunks that contain the straw golem, the exception occurs.

Here are my logs

The relevant parts of the stack trace are:

Caused by: java.lang.StackOverflowError
    at com.t2pellet.strawgolem.util.octree.Octree.subdivide(Octree.java:177) ~[Strawgolem-fabric-1.19.2-2.0.1-alpha.jar:?]
    at com.t2pellet.strawgolem.util.octree.Octree.insert(Octree.java:50) ~[Strawgolem-fabric-1.19.2-2.0.1-alpha.jar:?]
    at com.t2pellet.strawgolem.util.octree.Octree.insert(Octree.java:59) ~[Strawgolem-fabric-1.19.2-2.0.1-alpha.jar:?]
    at com.t2pellet.strawgolem.util.octree.Octree.insert(Octree.java:59) ~[Strawgolem-fabric-1.19.2-2.0.1-alpha.jar:?]
    at com.t2pellet.strawgolem.util.octree.Octree.insert(Octree.java:59) ~[Strawgolem-fabric-1.19.2-2.0.1-alpha.jar:?]

I am currently playing using the modpack Prominence I, however I can attempt to reproduce this issue without any mods, if you'd like.

I cannot identify which commit the version is from, as it does not seem to be tagged, however based on the curseforge description, I belive it is b2b3d2c. If I can get the exact commit this happens in, I can introduce some debug logging to print the structure of the OctTree causing this exception, to help with debugging. I'm not particularly sure what OctTree here is doing, so I can't really give any suggestions. Since I couldn't find the original commit, this is the (relevant) output my decompiler showed me:

   @Override
   public boolean insert(class_2338 pos) {
      if (!this.pointInBoundary(pos)) {
         return false;
      } else if (this.points.size() < 8 && this.northWestUp == null) {
         this.points.add(pos);
         return true;
      } else {
         if (this.northWestUp == null) {
            this.subdivide();
         }

         if (this.northWestUp.insert(pos)) {
            return true;
         } else if (this.northWestDown.insert(pos)) {
            return true;
         } else if (this.northEastUp.insert(pos)) {
            return true;
         } else if (this.northEastDown.insert(pos)) {
            return true;
         } else if (this.southWestUp.insert(pos)) {
            return true;
         } else if (this.southWestDown.insert(pos)) {
            return true;
         } else if (this.southEastUp.insert(pos)) {
            return true;
         } else {
            return this.southEastDown.insert(pos);
         }
      }
   }

perhaps the issue is ocurring with the

return this.southEastDown.insert(pos);

line? Unsure.

It seems that the source for OctTree has changed since then (I concretely identified this by looking at the findNearestFromList method, whose implementation differs from the current version)

Looking at different OctTree implementations, they seem to do some bounds checking to determine where to insert the point to.

Additionally, it seems other implementations use a max-depth, to avoid StackOverflowErrors. Perhaps this approach can be adopted here, with an upper limit of something like 64, or 128?

t2pellet commented 1 year ago

Thanks for this! Will take a look when I have time, a bit busy w uni and such atm, just got through midterm szn

t2pellet commented 1 year ago

Would you mind actually sending over a world that has this issue occuring? Just so I can reproduce better

solonovamax commented 1 year ago

Would you mind actually sending over a world that has this issue occuring? Just so I can reproduce better

tbh, I couldn't actually reproduce it a second time either, so idfk lol (though, I did downgrade the mod, so idk?) (but iirc upgrading it didn't cause it to happen again??)

though, if I had to guess, you could reproduce this by creating a massive area filled with crops, as from my understanding it's used to build a grid of crops the golem has access to.

solonovamax commented 1 year ago

I can try reproducing in a clean environment later, if you'd like.

t2pellet commented 1 year ago

I’ve updated the mod with some changes, and I believe theres an octree fix in there as well which might well fix this issue! It’s on modrinth and curse!

solonovamax commented 1 year ago

Also, the octree stuff causes the game to take an extremely long time while saving to disk, after having loaded many chunks

solonovamax commented 1 year ago

if you mean the commit f1c0eae990d075491de41f47390bd8acc85c05c8, I don't think that it will fix the stack overflow I was experiencing.

t2pellet commented 1 year ago

How big is your strawgolem.dat? Also if you could quantify “extremely long” for me.

the OctTrees store all the crops in the world, so it’s possible they’re just getting too big, but I don’t think it should be to the point where it’s really slowing down world saving.

I’ll take a look of course though.

I’ll try again to replicate the StackOverflow as well

solonovamax commented 1 year ago

How big is your strawgolem.dat? Also if you could quantify “extremely long” for me.

the OctTrees store all the crops in the world, so it’s possible they’re just getting too big, but I don’t think it should be to the point where it’s really slowing down world saving.

I’ll take a look of course though.

I’ll try again to replicate the StackOverflow as well

it makes it so that it will take around 15-30 seconds to save the world. I can check how large the file is later.

Also, I can attempt to reproduce in a clean env later.

t2pellet commented 1 year ago

I'm sorry I've been slow at dealing with this! I've been really swamped with everything recently.

The file size would help a lot just to verify what I believe is the cause of the problem!

Thanks for your diligence with this thus far :)

solonovamax commented 1 year ago

I'm sorry I've been slow at dealing with this! I've been really swamped with everything recently.

No worries!

The file size would help a lot just to verify what I believe is the cause of the problem!

I've removed the mod from my mods list, and can no longer find the strawgolem.dat file. Perhaps mc deleted it after removing the mod ?? (though, why would it do that???)

Abalieno commented 7 months ago

I bumped into this crash.

I've only added recently the mod, and not using anything from it. In fact it happened the exact moment I tried to place a Create fluid drain next to a basin. But cannot reproduce.

I thought it was related to some conflict with Create but maybe the crash was completely unrelated to what I was doing at the moment. I guess it could still be related to saving chunks in the background. But I still don't have anything from the mod directly active and only have a smallish area with flaxseed crops that I haven't touched in a while.

t2pellet commented 5 months ago

should be fixed in newest release (2.1.0-alpha)