SpongePowered / SpongeForge

A Forge mod that implements SpongeAPI
http://www.spongepowered.org/
MIT License
1.14k stars 306 forks source link

[Bug] Redstone neighbours update tracking result in StackOverflowError #3098

Open sandtechnology opened 4 years ago

sandtechnology commented 4 years ago

I am currently running

Issue Description Redstone casuing StackOverflowError, the redstone wires in this location as followed: TIM图片20200304010754.png

Look like it ONLY trigger when a mob active the pressure plate.

Updated: Reproduce with Only install Nucleus-1.14.2-S7.1-MC1.12.2

Updated: After troubleshooting and searching, I found the root cause is Sponge does not cancel neighbours update when cancelled ChangeBlockEvent (Related code in Nucleus), it also a performance issue to track this.

For Nucleus users, you can disable mob-griefing protection and use gamerule instead to fix it temporary.

full server log and crash-report (forced stop by watchdog):https://gist.github.com/sandtechnology/017ccb13c2c24fdcd7b347127ee80610

JBYoshi commented 4 years ago

Can reproduce on SF 4007, using the following listener (mostly copied from the Nucleus code linked above):

    @Listener
    @Exclude({ChangeBlockEvent.Grow.class, ChangeBlockEvent.Decay.class})
    public void onMobChangeBlock(ChangeBlockEvent event, @Root Living living) {
        if (living instanceof Player) {
            return;
        }

        // If the entity is not in the whitelist, then cancel the event.
        event.setCancelled(true);
    }
JBYoshi commented 4 years ago

What I've found so far:

  1. When a mob steps on the pressure plates (it may take some moving around for this to trigger), the pressure plate code first sets the block - without triggering any events - then manually causes a neighbor update.
  2. The redstone below the pressure plate receives the neighbor update. Seeing that the pressure plate above is active, it attempts to turn on.
  3. The redstone attempts to change its state in the world, which immediately fires an event. Since the event is cancelled, the power does not actually change.
  4. The redstone also queues itself and all neighboring redstone blocks for another update, and then sends neighbor updates to all of them.
  5. The neighbor updates repeat, passing between the various redstone wire blocks, until the stack overflows.
gabizou commented 4 years ago

@JBYoshi listen for the neighbor notification event, those ought to be cancelled as well, or if enhanced tracking is enabled, then it should very well be trying to discord the neighbor updates. There's a few gotchas with how Redstone works (as the issue is describing) and it comes down to block changes causing neighbor updates versus the blocks themselves receiving neighbor updates and the blocks calling to schedule more updates.

JBYoshi commented 4 years ago

So is this a problem on the plugin side? I copied my test code pretty much straight from Nucleus.

liampearson96 commented 4 years ago

@sandtechnology Hello I am currently having this issue on my server too. You say you can temporarily fix it by disabling mob griefing protection. You say this is a temporary fix? By that due you mean it stops it from happening for a certain amount of time or what? Any help would be great as this is currently what is preventing me from updating to the latest sponge on my server.

Thanks!

sandtechnology commented 4 years ago

Just is a workaround, I have no idea when they fixed it, and not sure started from which version will cause this.

Sent from Mailhttps://go.microsoft.com/fwlink/?LinkId=550986 for Windows 10

From: Liam Pearsonmailto:notifications@github.com Sent: 2020年4月28日 17:49 To: SpongePowered/SpongeForgemailto:SpongeForge@noreply.github.com Cc: sandtechnologymailto:a1294790523@hotmail.com; Mentionmailto:mention@noreply.github.com Subject: Re: [SpongePowered/SpongeForge] [Bug] Redstone neighbours update tracking result in StackOverflowError (#3098)

@sandtechnologyhttps://github.com/sandtechnology Hello I am currently having this issue on my server too. You say you can temporarily fix it by disabling mob griefing protection. You say this is a temporary fix? By that due you mean it stops it from happening for a certain amount of time or what? Any help would be great as this is currently what is preventing me from updating to the latest sponge on my server.

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/SpongePowered/SpongeForge/issues/3098#issuecomment-620500838, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AE3YYCZ33UFULB3RW4ZAEDTRO2REJANCNFSM4LAPCS3Q.

liampearson96 commented 4 years ago

@sandtechnology upon trying 'For Nucleus users, you can disable mob-griefing protection and use gamerule instead to fix it temporary' this no longer works. I can confirm the server will still stall even with these settings applied.

So still unknown.

sandtechnology commented 4 years ago

@sandtechnology upon trying 'For Nucleus users, you can disable mob-griefing protection and use gamerule instead to fix it temporary' this no longer works. I can confirm the server will still stall even with these settings applied.

So still unknown.

Other Plugin which listening ChangeBlockEvent and cancelled it will caused this, so the most best way is waiting the fix.