MovingBlocks / Terasology

Terasology - open source voxel world
http://terasology.org
Apache License 2.0
3.69k stars 1.34k forks source link

StackOverflow possible with circular references in prefabs #3739

Open Cervator opened 5 years ago

Cervator commented 5 years ago

What you were trying to do

(Edit: Updated way later with more details)

Put together a complex set of modules for play testing. Eventually figured out there was a circular reference in easterEgg.prefab in ShatteredPlanes but it would only trigger if you also had StructureTemplates enabled. Fixed with https://github.com/Terasology/ShatteredPlanes/pull/8

We should hopefully be able to catch and guard against such circular references at the engine level, bypassing the load of invalid prefabs and logging the issue clearly.

What actually happened

Crashed during initial world loading with a stackoverflow as the easterEgg.prefab was being processed, and effectively had a reference to itself.

How to reproduce

See this further down https://github.com/MovingBlocks/Terasology/issues/3739#issuecomment-608162476

Log details and game version

Latest develop with all modules at latest as of writing this

Note the first set of lines repeat quite a few times, and the stacktrace of course also keeps going for a while before the crash.

09:53:39.187 [main] INFO  o.t.e.prefab.internal.PrefabFormat - Attempting to deserialize prefab ShatteredPlanes:easterEgg with inputs [/ShatteredPlanes/assets/prefabs/easterEgg.prefab]
09:53:39.200 [main] INFO  o.t.e.prefab.internal.PrefabFormat - Attempting to deserialize prefab ShatteredPlanes:easterEgg with inputs [/ShatteredPlanes/assets/prefabs/easterEgg.prefab]
09:53:39.213 [main] INFO  o.t.e.prefab.internal.PrefabFormat - Attempting to deserialize prefab ShatteredPlanes:easterEgg with inputs [/ShatteredPlanes/assets/prefabs/easterEgg.prefab]
09:53:39.220 [main] ERROR o.terasology.engine.TerasologyEngine - Uncaught exception, attempting clean game shutdown
java.lang.StackOverflowError: null
    at java.io.FileOutputStream.writeBytes(Native Method)
    at java.io.FileOutputStream.write(Unknown Source)
    at java.io.BufferedOutputStream.flushBuffer(Unknown Source)
    at java.io.BufferedOutputStream.write(Unknown Source)
    at java.io.PrintStream.write(Unknown Source)
    at java.io.FilterOutputStream.write(Unknown Source)
    at ch.qos.logback.core.joran.spi.ConsoleTarget$1.write(ConsoleTarget.java:37)
    at ch.qos.logback.core.OutputStreamAppender.writeBytes(OutputStreamAppender.java:199)
    at ch.qos.logback.core.OutputStreamAppender.subAppend(OutputStreamAppender.java:231)
    at ch.qos.logback.core.OutputStreamAppender.append(OutputStreamAppender.java:102)
    at ch.qos.logback.core.UnsynchronizedAppenderBase.doAppend(UnsynchronizedAppenderBase.java:84)
    at ch.qos.logback.core.spi.AppenderAttachableImpl.appendLoopOnAppenders(AppenderAttachableImpl.java:51)
    at ch.qos.logback.classic.Logger.appendLoopOnAppenders(Logger.java:270)
    at ch.qos.logback.classic.Logger.callAppenders(Logger.java:257)
    at ch.qos.logback.classic.Logger.buildLoggingEventAndAppend(Logger.java:421)
    at ch.qos.logback.classic.Logger.filterAndLog_2(Logger.java:414)
    at ch.qos.logback.classic.Logger.info(Logger.java:587)
    at org.terasology.entitySystem.prefab.internal.PrefabFormat.load(PrefabFormat.java:53)
    at org.terasology.entitySystem.prefab.internal.PrefabFormat.load(PrefabFormat.java:36)
    at org.terasology.assets.module.UnloadedAssetData$AssetSourceResolver.load(UnloadedAssetData.java:292)
    at org.terasology.assets.module.UnloadedAssetData.load(UnloadedAssetData.java:180)
    at org.terasology.assets.module.ModuleAssetDataProducer.getAssetData(ModuleAssetDataProducer.java:228)
    at org.terasology.assets.AssetType.lambda$reload$2(AssetType.java:359)
    at java.security.AccessController.doPrivileged(Native Method)
    at org.terasology.assets.AssetType.reload(AssetType.java:357)
    at org.terasology.assets.AssetType.getNormalAsset(AssetType.java:386)
    at org.terasology.assets.AssetType.getAsset(AssetType.java:262)
    at org.terasology.assets.management.AssetManager.getAsset(AssetManager.java:223)
    at org.terasology.assets.management.AssetManager.getAsset(AssetManager.java:203)
    at org.terasology.assets.management.AssetManager.getAsset(AssetManager.java:187)
    at org.terasology.utilities.Assets.get(Assets.java:80)
    at org.terasology.utilities.Assets.getPrefab(Assets.java:274)
    at org.terasology.persistence.typeHandling.extensionTypes.PrefabTypeHandler.getFromString(PrefabTypeHandler.java:43)
    at org.terasology.persistence.typeHandling.extensionTypes.PrefabTypeHandler.getFromString(PrefabTypeHandler.java:25)
    at org.terasology.persistence.typeHandling.StringRepresentationTypeHandler.deserialize(StringRepresentationTypeHandler.java:37)
    at org.terasology.persistence.typeHandling.TypeHandler.deserializeOrNull(TypeHandler.java:68)
    at org.terasology.persistence.typeHandling.Serializer.deserializeOnto(Serializer.java:96)
    at org.terasology.persistence.typeHandling.Serializer.deserializeOnto(Serializer.java:153)
    at org.terasology.persistence.serializers.ComponentSerializer.deserializeOnto(ComponentSerializer.java:197)
    at org.terasology.persistence.serializers.ComponentSerializer.deserialize(ComponentSerializer.java:114)
    at org.terasology.persistence.serializers.PrefabSerializer.applyComponentChanges(PrefabSerializer.java:164)
    at org.terasology.persistence.serializers.PrefabSerializer.deserialize(PrefabSerializer.java:136)
    at org.terasology.persistence.serializers.PrefabSerializer.deserialize(PrefabSerializer.java:116)
    at org.terasology.entitySystem.prefab.internal.PrefabFormat.load(PrefabFormat.java:55)
    at org.terasology.entitySystem.prefab.internal.PrefabFormat.load(PrefabFormat.java:36)
    at org.terasology.assets.module.UnloadedAssetData$AssetSourceResolver.load(UnloadedAssetData.java:292)
    at org.terasology.assets.module.UnloadedAssetData.load(UnloadedAssetData.java:180)
    at org.terasology.assets.module.ModuleAssetDataProducer.getAssetData(ModuleAssetDataProducer.java:228)
    at org.terasology.assets.AssetType.lambda$reload$2(AssetType.java:359)
    at java.security.AccessController.doPrivileged(Native Method)
    at org.terasology.assets.AssetType.reload(AssetType.java:357)
    at org.terasology.assets.AssetType.getNormalAsset(AssetType.java:386)
    at org.terasology.assets.AssetType.getAsset(AssetType.java:262)
    at org.terasology.assets.management.AssetManager.getAsset(AssetManager.java:223)
    at org.terasology.assets.management.AssetManager.getAsset(AssetManager.java:203)
    at org.terasology.assets.management.AssetManager.getAsset(AssetManager.java:187)
    at org.terasology.utilities.Assets.get(Assets.java:80)
    at org.terasology.utilities.Assets.getPrefab(Assets.java:274)
    at org.terasology.persistence.typeHandling.extensionTypes.PrefabTypeHandler.getFromString(PrefabTypeHandler.java:43)
    at org.terasology.persistence.typeHandling.extensionTypes.PrefabTypeHandler.getFromString(PrefabTypeHandler.java:25)
    at org.terasology.persistence.typeHandling.StringRepresentationTypeHandler.deserialize(StringRepresentationTypeHandler.java:37)
    at org.terasology.persistence.typeHandling.TypeHandler.deserializeOrNull(TypeHandler.java:68)
    at org.terasology.persistence.typeHandling.Serializer.deserializeOnto(Serializer.java:96)
    at org.terasology.persistence.typeHandling.Serializer.deserializeOnto(Serializer.java:153)
    at org.terasology.persistence.serializers.ComponentSerializer.deserializeOnto(ComponentSerializer.java:197)
    at org.terasology.persistence.serializers.ComponentSerializer.deserialize(ComponentSerializer.java:114)
    at org.terasology.persistence.serializers.PrefabSerializer.applyComponentChanges(PrefabSerializer.java:164)
    at org.terasology.persistence.serializers.PrefabSerializer.deserialize(PrefabSerializer.java:136)
eviltak commented 5 years ago

Looks like a recursive Asset/Prefab dependency (A component in prefab A refers to prefab B, and a component in prefab B refers to prefab A, or a longer loop). We could either modify the asset to remove the recursion, or we could add lazy or late loading of prefabs where we resolve the prefab using an Asset.get call only after all prefabs have been loaded.

Cervator commented 4 years ago

Hit it by chance again and reminded myself that the affected asset seems pretty clear even based on the log snippet above: easterEgg.prefab in ShatteredPlanes refers to itself as a StructureTemplate, which is kinda weird as it doesn't actually depend on the ST module. I think that the error only happens if the ST module is also enabled, as otherwise that part of the prefab just gets ignored (as the game doesn't know what an ST is if its module isn't enabled)

It would be nice to find a way to harden the engine to simply log a warning for a circular reference rather than crash. Although at least it crashes on startup not at some arbitrary point later in a game session.

To replicate: Define an asset that ends up referring to itself. Example easterEgg.prefab in ShatteredPlanes (adjust naming as needed for a different example):

{
  "StructureTemplate": {
    "type": "ShatteredPlanes:easterEgg"
  }
}