NPE on character movement extrapolation when killing a sheep

[jdrueckert]

General Info

Terasology Version: https://jenkins.terasology.io/teraorg/job/Terasology/job/Omega/job/develop/409/ Multiplayer Setup with headless (official) Playtest Server

What you were trying to do

Kill a sheep in MetalRenegades

What actually happened

Server crashed (ran into NPE).

How to reproduce

1. Join MetalRenegades world hosted on headless server
2. Continuously hit a sheep to kill it
3. See server crash

Log details

java.lang.NullPointerException: null
        at org.terasology.engine.logic.characters.CharacterMovementSystemUtility.extrapolateCharacterMovementComponent(CharacterMovementSystemUtility.java:120)
        at org.terasology.engine.logic.characters.CharacterMovementSystemUtility.setToExtrapolateState(CharacterMovementSystemUtility.java:106)
        at org.terasology.engine.logic.characters.ServerCharacterPredictionSystem.setToTime(ServerCharacterPredictionSystem.java:243)
        at org.terasology.engine.logic.characters.ServerCharacterPredictionSystem.restoreToPresent(ServerCharacterPredictionSystem.java:263)
        at org.terasology.engine.network.internal.NetClient.processEvents(NetClient.java:540)
        at org.terasology.engine.network.internal.NetClient.processReceivedMessages(NetClient.java:421)
        at org.terasology.engine.network.internal.NetClient.update(NetClient.java:219)
        at org.terasology.engine.network.internal.NetworkSystemImpl.update(NetworkSystemImpl.java:323)
        at org.terasology.engine.core.subsystem.common.NetworkSubsystem.preUpdate(NetworkSubsystem.java:37)
        at org.terasology.engine.core.TerasologyEngine.tick(TerasologyEngine.java:492)
        at org.terasology.engine.core.TerasologyEngine.mainLoop(TerasologyEngine.java:459)
        at org.terasology.engine.core.TerasologyEngine.runMain(TerasologyEngine.java:435)
        at org.terasology.engine.core.TerasologyEngine.run(TerasologyEngine.java:401)
        at org.terasology.engine.Terasology.call(Terasology.java:180)
        at org.terasology.engine.Terasology.call(Terasology.java:69)
        at picocli.CommandLine.executeUserObject(CommandLine.java:1933)
        at picocli.CommandLine.access$1200(CommandLine.java:145)
        at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2332)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2326)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2291)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2159)
        at picocli.CommandLine.execute(CommandLine.java:2058)
        at org.terasology.engine.Terasology.main(Terasology.java:138)

Additional Infos / Context

Current assumption: The sheep is theoretically already dead when the ServerCharacterPredictionSystem tries to update entities and triggers setToExtrapolateState. However, because we're waiting for the dying animation to finish, the entity is still present in the characterStates map and is included in the update. When attempting to access CharacterMovementComponent for the extrapolation, the server runs into an NPE because the component has already been removed.

[jdrueckert]

Theoretically, I would read https://github.com/MovingBlocks/Terasology/blob/8e265d0e08b2c2ee64b391ee7dfbebc1203551c8/engine/src/main/java/org/terasology/engine/logic/characters/ServerCharacterPredictionSystem.java#L97-L103 as "before the component is removed, the entity is added to the characterStatesToRemove list that will be used in the next update() to remove all listed entities from the characterStates map, which is the map that is walked through to find entities whose state needs to be extrapolated in https://github.com/MovingBlocks/Terasology/blob/develop/engine/src/main/java/org/terasology/engine/logic/characters/CharacterMovementSystemUtility.java#L99-L109 .

Practically, if this is true, the only way I could've run into the issue is probably a race condition :thinking: