AppleSkin

AppleSkin

236M Downloads

Saturation hud overlay causing Out of Memory error

HugeLol556 opened this issue · 19 comments

commented

During the "Saving World" phase of loading into a world, saturation hud overlay causes Out of Memory error due to too large array request. Doesn't crash the game, just pops up a menu where Minecraft warns you of the event and allows you to quit or return to menu.
Fixed by setting showSaturationHudOverlay to false in config.
Issue found while trying to join a server using the modpack ATM6 1.7.1.

Call stack:

[19Jul2021 12:05:30.534] [Render thread/FATAL] [net.minecraft.client.Minecraft/]: Out of memory
java.lang.OutOfMemoryError: Requested array size exceeds VM limit
	at java.util.Arrays.copyOf(Unknown Source) ~[?:1.8.0_291]
	at java.util.Vector.grow(Unknown Source) ~[?:1.8.0_291]
	at java.util.Vector.ensureCapacityHelper(Unknown Source) ~[?:1.8.0_291]
	at java.util.Vector.setSize(Unknown Source) ~[?:1.8.0_291]
	at squeek.appleskin.client.HUDOverlayHandler.generateBarOffsets(HUDOverlayHandler.java:494) ~[appleskin:mc1.16.4-2.0.0]
	at squeek.appleskin.client.HUDOverlayHandler.onRender(HUDOverlayHandler.java:107) ~[appleskin:mc1.16.4-2.0.0]
	at net.minecraftforge.eventbus.ASMEventHandler_3056_HUDOverlayHandler_onRender_Post.invoke(.dynamic) ~[?:?]
	at net.minecraftforge.eventbus.ASMEventHandler.invoke(ASMEventHandler.java:85) ~[eventbus-4.0.0.jar:?]
	at net.minecraftforge.eventbus.EventBus$$Lambda$2612/1344076285.invoke(Unknown Source) ~[?:?]
	at net.minecraftforge.eventbus.EventBus.post(EventBus.java:302) ~[eventbus-4.0.0.jar:?]
	at net.minecraftforge.eventbus.EventBus.post(EventBus.java:283) ~[eventbus-4.0.0.jar:?]
	at net.minecraftforge.client.gui.ForgeIngameGui.post(ForgeIngameGui.java:832) ~[forge:?]
	at net.minecraftforge.client.gui.ForgeIngameGui.renderFood(ForgeIngameGui.java:545) ~[forge:?]
	at net.minecraftforge.client.gui.ForgeIngameGui.func_238445_a_(ForgeIngameGui.java:165) ~[forge:?]
	at net.minecraft.client.renderer.GameRenderer.func_195458_a(GameRenderer.java:472) ~[?:?]
	at net.minecraft.client.Minecraft.func_195542_b(Minecraft.java:976) ~[?:?]
	at net.minecraft.client.Minecraft.func_99999_d(Minecraft.java:607) ~[?:?]
	at net.minecraft.client.main.Main.main(Main.java:184) ~[minecraft-1.16.5-client.jar:?]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_291]
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_291]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_291]
	at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_291]
	at net.minecraftforge.fml.loading.FMLClientLaunchProvider.lambda$launchService$0(FMLClientLaunchProvider.java:51) ~[forge-1.16.5-36.1.32-launcher.jar:36.1]
	at net.minecraftforge.fml.loading.FMLClientLaunchProvider$$Lambda$503/741236338.call(Unknown Source) ~[?:?]
	at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:37) ~[modlauncher-8.0.9.jar:?]
	at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:54) ~[modlauncher-8.0.9.jar:?]
	at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:72) ~[modlauncher-8.0.9.jar:?]
	at cpw.mods.modlauncher.Launcher.run(Launcher.java:82) ~[modlauncher-8.0.9.jar:?]
	at cpw.mods.modlauncher.Launcher.main(Launcher.java:66) ~[modlauncher-8.0.9.jar:?]
	at io.github.zekerzhayard.forgewrapper.installer.Main.main(Main.java:50) ~[ForgeWrapper-1.4.2.jar:?]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_291]
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_291]
commented

Works great, thank you!

commented

Thanks for reporting, this sounds like a bad one. Will try to get it fixed soon.

EDIT: These are the lines in question:

// adjust the size
if (healthBarOffsets.size() != healthBars)
healthBarOffsets.setSize(healthBars);

commented

mod is gived an infinite health status of the player?

commented

No that isn't it, at least I don't have any active effect that would do that. It might be absorption, as I have 2 bonus hearts but I don't know what mod gives them to me. I was also wrong about the options, it wasn't the saturation overlay. I just disabled a bunch more to fix it and now I can't see my health except for the two bonus ones. So whatever setting disabled hearts seems to be the fix.

commented

can your provide a screenshot of the game?

i tried to duplicate it by modifying generic.max_health, but it didn't

commented

@HugeLol556 could you install this modified version and post the crash that happens when using it (you'll need to turn on health bar rendering again):

https://www.ryanliptak.com/misc/appleskin-forge-mc1.16.4-2.0.0+healthtest1.jar

commented

crash-2021-07-19_19.41.40-client.txt
Tried twice, both attempts showed similar data.

commented

Thanks, so it looks like the values are normal at some point. Try this one now, please:

https://www.ryanliptak.com/misc/appleskin-forge-mc1.16.4-2.0.0+healthtest2.jar

commented

@squeek502 I think it would be better to add a maximum limit

diff --git a/java/squeek/appleskin/client/HUDOverlayHandler.java b/java/squeek/appleskin/client/HUDOverlayHandler.java
index ca82ff3..dab7051 100644
--- a/java/squeek/appleskin/client/HUDOverlayHandler.java
+++ b/java/squeek/appleskin/client/HUDOverlayHandler.java
@@ -2,10 +2,8 @@ package squeek.appleskin.client;
 
 import com.mojang.blaze3d.matrix.MatrixStack;
 import com.mojang.blaze3d.systems.RenderSystem;
-import com.sun.org.apache.xpath.internal.operations.Mod;
 import net.minecraft.client.Minecraft;
 import net.minecraft.client.gui.AbstractGui;
-import net.minecraft.client.gui.screen.Screen;
 import net.minecraft.entity.ai.attributes.Attributes;
 import net.minecraft.entity.player.PlayerEntity;
 import net.minecraft.item.ItemStack;
@@ -308,6 +306,10 @@ public class HUDOverlayHandler
 
                for (int i = startHealthBars; i < endHealthBars; ++i)
                {
+                       // health maybe too much to can't display
+                       if (i >= healthBarOffsets.size())
+                               break;
+
                        // gets the offset that needs to be render of icon
                        IntPoint offset = healthBarOffsets.get(i);
                        if (offset == null)
@@ -457,14 +459,19 @@ public class HUDOverlayHandler
 
        private void generateBarOffsets(int top, int left, int right, int ticks, PlayerEntity player)
        {
+               // only display first 1024 hearts
+               final int preferMaximumHealthBars = 1024;
+
                final int preferHealthBars = 10;
                final int preferFoodBars = 10;
 
-               final float maxHealth = player.getMaxHealth();
+               // due mixin of player.getMaxHealth() and player.getAttributeValue(Attributes.MAX_HEALTH) maybe is different,
+               // so we keep the same code as `InGameGUI.java`
+               final float maxHealth = (float)player.getAttributeValue(Attributes.MAX_HEALTH);
                final float absorptionHealth = (float)Math.ceil(player.getAbsorptionAmount());
 
                int healthBars = (int)Math.ceil((maxHealth + absorptionHealth) / 2.0F);
-               int healthRows = (int)Math.ceil((float)healthBars / 10.0F);
+               int healthRows = (int)Math.ceil((float)healthBars / (float)preferHealthBars);
 
                int healthRowHeight = Math.max(10 - (healthRows - 2), 3);
 
@@ -486,12 +493,12 @@ public class HUDOverlayHandler
                        shouldAnimatedHealth = Math.ceil(player.getHealth()) <= 4;
                }
 
-               // hard code in `InGameHUD`
+               // hard code in `InGameGUI.java`
                random.setSeed((long)(ticks * 312871));
 
                // adjust the size
-               if (healthBarOffsets.size() != healthBars)
-                       healthBarOffsets.setSize(healthBars);
+               if (healthBarOffsets.size() != Math.min(healthBars, preferMaximumHealthBars))
+                       healthBarOffsets.setSize(Math.min(healthBars, preferMaximumHealthBars));
 
                if (foodBarOffsets.size() != preferFoodBars)
                        foodBarOffsets.setSize(preferFoodBars);
@@ -506,6 +513,10 @@ public class HUDOverlayHandler
                        if (shouldAnimatedHealth)
                                y += random.nextInt(2);
 
+                       // random still evaluate, but can't add to offsets
+                       if (i >= preferMaximumHealthBars)
+                               continue;
+
                        // reuse the point object to reduce memory usage
                        IntPoint point = healthBarOffsets.get(i);
                        if (point == null)
commented

Well, my friend deleted the serverstarter.lock file and restarted the server. I haven't been able to replicate the crash since. It seems some strange corruption was only affecting me.

I'll continue to use healthtest2 just incase this ever appears again.

Sorry about this, thanks for your help.

commented

No worries, thanks for being willing to test things for us!

It probably makes sense for us to put in a better error message in the mod just in case this type of thing happens again. This is the code I put in healthtest2 for future reference:

		// adjust the size
		if (healthBarOffsets.size() != healthBars)
		{
			try
			{
				healthBarOffsets.setSize(healthBars);
			}
			catch(Exception e)
			{
				throw new RuntimeException("Tried and failed to set healthBarOffsets size to " + healthBars + " from max health="+maxHealth+" and absorption="+absorptionHealth);
			}
		}
commented

I think I'd prefer a crash, since this seems like it has to be either:

  • A bug in some other mod that should be reported/fixed rather than worked around
  • Something real that we'd need to add proper support for (infinite health or something?). It seems like this would cause problems with Vanilla's health bar rendering, though

If we clamp the health bars to avoid the crash, then we'd be doing the incorrect thing in either case. If we crash, then we'd have a chance at fixing it properly.

commented

sounds like is good.

i suggest change player.getMaxHealth to player.getAttributeValue, in older versions player.getMaxHealth might have been hooked, which caused we calculations to maybe is different with Vanilla's.

-               final float maxHealth = player.getMaxHealth();
+               // due mixin of player.getMaxHealth() and player.getAttributeValue(Attributes.MAX_HEALTH) maybe is different,
+               // so we keep the same code as `InGameGUI.java`
+               final float maxHealth = (float)player.getAttributeValue(Attributes.MAX_HEALTH);
commented

Hello again, new development. The error started again but wasn't being caught by healthtest2 (turns out since it is an OutOfMemory_Error_, catching an exception doesn't grab it). I fixed the catch and got this line:

"Tried and failed to set healthBarOffsets size to 2147483647 from max health=24.0 and absorption=Infinity"

If I successfully load into the server I do not actually have infinite absorption and I'm not sure where this information could be coming from.

I won't have access to my computer for some time but hopefully this helps a bit and I'll try more debugging when I'm available.

// new catch
catch(OutOfMemoryError e)
commented

Nice work, thanks for the info!

Sounds like the HUD might be getting rendered between loading into the server and the player's data getting synced, so the uninitialized client data is being used for a bit and for whatever reason that happens to work out to infinity?

What's confusing is why this isn't a problem for the vanilla HUD rendering, since presumably they are iterating from infinity in their render icons loop...? Maybe we're doing our render logic when the vanilla HUD isn't and we need an extra check somewhere to avoid this erroneous call?

commented

Ohhhhh, I think I understand what's happening. We're still only listening for RenderGameOverlayEvent.ElementType.FOOD and rendering both food and health there. Instead, we need to break things up into RenderGameOverlayEvent.ElementType.FOOD and RenderGameOverlayEvent.ElementType.HEALTH.

commented

Interestingly the health value is correct, 20 plus 4 bonus. But the absorption has some weird discrepancy.

commented

Okay, my hypothesis in #127 (comment) was wrong. Rendering the health bar during RenderGameOverlayEvent.ElementType.FOOD is slightly incorrect, but it's unrelated to this, since the health rendering is always called before the hunger bar rendering (and the health bar is always rendered if the hunger bar is, so the scenario I had in mind isn't possible AFAIK).

Turns out that the vanilla health rendering handles the infinite absorption case mostly by accident:

  • player.getAbsorptionAmount() returns Float.POSITIVE_INFINITY
  • MathHelper.ceil(player.getAbsorptionAmount()) returns -2.14748365E9
  • The icon render loop is for (int i = MathHelper.ceil((healthMax + absorb) / 2.0F) - 1; i >= 0; --i) where the initial value of i works out to -1073741825 which is less than 0 so the loop exits immediately (meaning no icons are rendered at all, which lines up with what you were seeing earlier)

So maybe there's a mod in ATM6 that is intentionally setting absorption to infinity while people load into the multiplayer server (to stop deaths while loading)?

No matter what is actually causing it, though, it seems like we'll need to handle infinite absorption as a special case. Probably should just not render anything if we get infinite absorption.

commented

Should be fixed by 4418b96. Here's a build if you want to try it out and make sure it fixes it for you:

https://www.ryanliptak.com/misc/appleskin-forge-mc1.16.4-2.0.0+healthtest3.jar