New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardening of some sort needed in DelayedActionSystem (plant growth can cause NPE crashes?) #2473

Open
Cervator opened this Issue Sep 10, 2016 · 1 comment

Comments

Projects
None yet
3 participants
@Cervator
Member

Cervator commented Sep 10, 2016

During multiplayer event the server crashed a minute after @rzats had himself crashed out via Terasology/JoshariasSurvival#14 shortly after I crashed out via #2361

I suspect something about the blueberry bush growth stages from JoshariasSurvival, which is based off SimpleFarming which in turn uses the DelayManager system from the engine. If I were to wager a guess there was an immature blueberry bush in a chunk one of us had been in that had its growth tick in a way that was no longer valid with no players left on the server.

Crash happened in DelayedActionSystem and while the log doesn't identify exactly what the scheduled action was I think a fair bet would be a blueberry bush. That in turn is defined in BlueberrySeed.prefab from JoshariasSurvival, which has a PlantDefinitionComponent that still has a bit of logic and other fanciness despite being a Component (old code I suspect). The related logic system is FarmingAuthoritySystem which is what in turn uses the DelayManager.

So some things to consider looking at:

  • Find out what causes the (likely) blueberry growth to result in an invalid scheduled action (something goes stale as player(s) disappear?)
  • Harden DelayedActionSystem to where it has error handling for encountering invalid scheduled actions (expect that invalid tasks could be created somehow)
  • Log some better details when a bad action is encountered (which one is it? Where did it come from?)
  • Look at SimpleFarming's use of delayed actions (probably predates the refactoring @xrtariq2594 applied to the delay system and wouldn't have been an obvious thing to test)
  • Simplify PlantDefinitionComponent also from SimpleFarming to follow code guidelines (no logic in Components, keep them basic)
  • Bonus: Add some more documentation to the scheduled task system, maybe some basic examples or even a little tutorial somewhere?
  • Edit addition: It seems fairly reproducible in multiplayer that breaking a blueberry bush (after picking the berries?) can cause a crash. Maybe not directly related, could be more about #2361

Error log below (server side):

14:01:59.002 [main] WARN  o.t.l.c.ServerCharacterPredictionSystem - Received too much input from EntityRef{id = 9050, netId = 20052, prefab = 'engine:player'}, dropping input.
14:01:59.007 [main] WARN  o.t.l.c.ServerCharacterPredictionSystem - Received too much input from EntityRef{id = 9050, netId = 20052, prefab = 'engine:player'}, dropping input.
14:01:59.013 [main] WARN  o.t.l.c.ServerCharacterPredictionSystem - Received too much input from EntityRef{id = 9050, netId = 20052, prefab = 'engine:player'}, dropping input.
14:02:07.843 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: rzats
14:02:08.237 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: rzats
14:02:08.726 [main] INFO  o.t.p.i.ReadWriteStorageManager - Saving - Creating game snapshot
14:02:08.727 [main] INFO  o.t.p.i.ReadWriteStorageManager - Saving - Snapshot created: Writing phase starts
14:02:11.916 [main] INFO  o.t.p.i.ReadWriteStorageManager - Saving - Creating game snapshot
14:02:11.916 [main] INFO  o.t.p.i.ReadWriteStorageManager - Saving - Snapshot created: Writing phase starts
14:03:05.293 [main] ERROR o.terasology.engine.TerasologyEngine - Uncaught exception, attempting clean game shutdown
java.lang.NullPointerException: null
    at org.terasology.logic.delay.DelayedActionSystem.lambda$invokeDelayedOperations$271(DelayedActionSystem.java:78)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
    at java.util.LinkedList$LLSpliterator.forEachRemaining(LinkedList.java:1235)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
    at org.terasology.logic.delay.DelayedActionSystem.invokeDelayedOperations(DelayedActionSystem.java:75)
    at org.terasology.logic.delay.DelayedActionSystem.update(DelayedActionSystem.java:58)
    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)
    at org.terasology.engine.Terasology.main(Terasology.java:141)
14:03:05.294 [main] INFO  o.terasology.engine.TerasologyEngine - Shutting down Terasology...
14:03:05.306 [main] INFO  o.t.p.i.ReadWriteStorageManager - Saving - Creating game snapshot
14:03:05.307 [main] INFO  o.t.p.i.ReadWriteStorageManager - Saving - Snapshot created: Writing phase starts
14:03:05.322 [main] INFO  o.t.n.internal.NetworkSystemImpl - Network shutdown

@xrtariq2594

This comment has been minimized.

Show comment
Hide comment
@xrtariq2594

xrtariq2594 Sep 16, 2016

Contributor

Item 5 has been improved in the develop branch of SimpleFarming.

Contributor

xrtariq2594 commented Sep 16, 2016

Item 5 has been improved in the develop branch of SimpleFarming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment