Sending CharacterMoveInputEvent from BT Node is unsafe

[kaen]

What you were trying to do

Move a character from a behavior tree Node

What actually happened

Crashed with concurrent modification exception

How to reproduce

Create a behavior node that moves an entity fast enough to take collision damage, and allow it to kill itself in this process.

I discovered this while having very fast deer run off the side of a hill.

Log details and game version

I was able to capture the problem with a wach point in IntelliJ, getting the bug to occur takes a couple minutes but always happens under the conditions described above eventually. More generally, destroying any entity with a Behavior component from within a behavior Node will trigger a similar Exception.

As you can see below, we modify the interpreter list while iterating through it:

https://github.com/MovingBlocks/Terasology/blob/develop/engine/src/main/java/org/terasology/logic/behavior/BehaviorSystem.java#L100
https://github.com/MovingBlocks/Terasology/blob/develop/engine/src/main/java/org/terasology/logic/behavior/BehaviorSystem.java#L107

"main@1" prio=10 tid=0x1 nid=NA runnable
  java.lang.Thread.State: RUNNABLE
      at org.terasology.logic.behavior.BehaviorSystem.onBehaviorDeactivated(BehaviorSystem.java:100)
      at org.terasology.logic.behavior.BehaviorSystemMethodAccess.invoke(Unknown Source:-1)
      at org.terasology.entitySystem.event.internal.EventSystemImpl$ByteCodeEventHandlerInfo.invoke(EventSystemImpl.java:506)
      at org.terasology.entitySystem.event.internal.EventSystemImpl.sendStandardEvent(EventSystemImpl.java:269)
      at org.terasology.entitySystem.event.internal.EventSystemImpl.send(EventSystemImpl.java:260)
      at org.terasology.entitySystem.entity.internal.PojoEntityManager.destroy(PojoEntityManager.java:538)
      at org.terasology.entitySystem.entity.internal.BaseEntityRef.destroy(BaseEntityRef.java:138)
      at org.terasology.logic.health.EntityDestructionAuthoritySystem.onDestroy(EntityDestructionAuthoritySystem.java:32)
      at org.terasology.logic.health.EntityDestructionAuthoritySystemMethodAccess.invoke(Unknown Source:-1)
      at org.terasology.entitySystem.event.internal.EventSystemImpl$ByteCodeEventHandlerInfo.invoke(EventSystemImpl.java:506)
      at org.terasology.entitySystem.event.internal.EventSystemImpl.sendStandardEvent(EventSystemImpl.java:269)
      at org.terasology.entitySystem.event.internal.EventSystemImpl.send(EventSystemImpl.java:260)
      at org.terasology.entitySystem.entity.internal.BaseEntityRef.send(BaseEntityRef.java:145)
      at org.terasology.logic.health.HealthAuthoritySystem.doDamage(HealthAuthoritySystem.java:185)
      at org.terasology.logic.health.HealthAuthoritySystem.checkDamage(HealthAuthoritySystem.java:200)
      at org.terasology.logic.health.HealthAuthoritySystem.onLand(HealthAuthoritySystem.java:241)
      at org.terasology.logic.health.HealthAuthoritySystemMethodAccess.invoke(Unknown Source:-1)
      at org.terasology.entitySystem.event.internal.EventSystemImpl$ByteCodeEventHandlerInfo.invoke(EventSystemImpl.java:506)
      at org.terasology.entitySystem.event.internal.EventSystemImpl.sendStandardEvent(EventSystemImpl.java:269)
      at org.terasology.entitySystem.event.internal.EventSystemImpl.send(EventSystemImpl.java:260)
      at org.terasology.entitySystem.entity.internal.BaseEntityRef.send(BaseEntityRef.java:145)
      at org.terasology.logic.characters.KinematicCharacterMover.walk(KinematicCharacterMover.java:647)
      at org.terasology.logic.characters.KinematicCharacterMover.updatePosition(KinematicCharacterMover.java:559)
      at org.terasology.logic.characters.KinematicCharacterMover.step(KinematicCharacterMover.java:103)
      at org.terasology.logic.characters.ServerCharacterPredictionSystem.stepState(ServerCharacterPredictionSystem.java:168)
      at org.terasology.logic.characters.ServerCharacterPredictionSystem.onPlayerInput(ServerCharacterPredictionSystem.java:126)
      at org.terasology.logic.characters.ServerCharacterPredictionSystemMethodAccess.invoke(Unknown Source:-1)
      at org.terasology.entitySystem.event.internal.EventSystemImpl$ByteCodeEventHandlerInfo.invoke(EventSystemImpl.java:506)
      at org.terasology.entitySystem.event.internal.EventSystemImpl.sendStandardEvent(EventSystemImpl.java:269)
      at org.terasology.entitySystem.event.internal.EventSystemImpl.send(EventSystemImpl.java:260)
      at org.terasology.entitySystem.entity.internal.BaseEntityRef.send(BaseEntityRef.java:145)
      at org.terasology.flexiblemovement.plugin.WalkingMovementPlugin.move(WalkingMovementPlugin.java:64)
      at org.terasology.flexiblemovement.MoveToNode$MoveToNodeTask.update(MoveToNode.java:62)
      at org.terasology.logic.behavior.tree.Task.tick(Task.java:90)
      at org.terasology.logic.behavior.tree.Interpreter.step(Interpreter.java:181)
      at org.terasology.logic.behavior.tree.Interpreter.tick(Interpreter.java:154)
      at org.terasology.logic.behavior.BehaviorSystem.update(BehaviorSystem.java:107)
      at org.terasology.engine.modes.StateIngame.update(StateIngame.java:176)
      at org.terasology.engine.TerasologyEngine.mainLoop(TerasologyEngine.java:413)
      at org.terasology.engine.TerasologyEngine.run(TerasologyEngine.java:368)
      - locked <0x198c> (a org.terasology.engine.TerasologyEngine)
      at org.terasology.engine.Terasology.main(Terasology.java:152)

[Cervator]

I suspect this issue might be out of date with the BT overhaul merged as part of engine v2. If @kaen here or anybody else can confirm/deny please do so we can find out whether it is safe to close this issue :-)