Infinite loop on ChunkLoadEvent.getChunk().getEntities()

velnias75 commented 3 years ago

Expected behavior

No infinite loop

Observed/Actual behavior

Can't move further into the affected chunk.

Steps/models to reproduce

hard to say, I suspect it is a race condition May be introduced by the fix of [SPIGOT-6547] getEntities() not working as expected.

Plugin list

Plugins (11): DiscordSRV, dynmap, Images, JoinMessage, LuckPerms, ProtocolLib, SetHome*, spark, ViewDistanceTweaks, VipSlot*,XItemsRemover

The bold plugin was causing the infinite loop, but was working well before.

Paper version

This server is running Paper version git-Paper-241 (MC: 1.17.1) (Implementing API version 1.17.1-R0.1-SNAPSHOT) (Git: 1276bd5) You are running the latest version Previous version: git-Paper-234 (MC: 1.17.1)

Agreements

[X] I am running the latest version of Paper available from https://papermc.io/downloads.
[X] I have searched for and ensured there isn't already an open issue regarding this.
[X] My version of Minecraft is supported by Paper.

Other

No response

Machine-Maker commented 3 years ago

Can you try this on build 238? It looks like that's the logic upstream just merged.

electronicboy commented 3 years ago

This is directly cuased by the upstream, getEntities now calls into the await logic which is used to drain the chunk processing queues, etc, It doesn't even look like it NEEDS to use await there...

velnias75 commented 3 years ago

Can you try this on build 238? It looks like that's the logic upstream just merged.

I could try, but I have no clue to reproduce it again. I would need to find a chunk with exactly all conditions which trigger that bug. I've already checked all reachable chunk candidates with no avail. The first offending chunk is working again after a server restart.

So I suspect it either a race condition or the bug triggers only on very strange circumstances.

This is the code of the triggering plugin: https://github.com/Xezard/XItemsRemover/blob/9f77ee43c47ec0807a80242b3737e80ad0480d7a/src/main/java/ru/xezard/items/remover/listeners/ChunkLoadListener.java (Line 38)

electronicboy commented 3 years ago

while (!entityManager.areEntitiesLoaded(pair)) {
            entityManager.tick();
            if (entityManager.areEntitiesLoaded(pair))
                break;
            java.util.concurrent.locks.LockSupport.parkNanos("entityAwait", 1000L);
        }

is a 2 seconds of brain what I'd replace the managedBlock in CraftChunk with, I don't see why upstream is using the managed block unless their aim is to try to allow it to process chunks, but, that creates its own set of headaches, but, also need to look into the unloading order here of entities too, so, this is potentially not safe yet, nor tested at all

electronicboy commented 3 years ago

then again, CB already explictly tells the entity system that those entities should load, so, that actually looks fine

velnias75 commented 3 years ago

Any idea for a workaround? I've already forked the plugin for a past PR. Tomorrow and Sunday is playtime with a friend and I would like to have a working server without downgrading.

electronicboy commented 3 years ago

There is no workaround without not calling getChunkEntities until the ChunkLoadEvent fires, otherwise, if you can mod the server, try the code above

PureGero commented 3 years ago

You could try using EntitiesLoadEvent instead of ChunkLoadEvent

velnias75 commented 3 years ago

You could try using EntitiesLoadEvent instead of ChunkLoadEvent

Since we need a stable Server on the weekend I'll wait until Monday if the paper-Team will find a solution.

PaperMC / Paper