WorldBorder

WorldBorder

1M Downloads

File access in main thread can cause server to hang

RoboMWM opened this issue ยท 4 comments

commented

Seems WorldBorder accesses the region files on the main thread. https://gist.github.com/RoboMWM/ade8e2b696eff6e65310de405c0ad2b8

This becomes an issue on already-partially-generated worlds, where many chunks have already been generated. (And also slow HDDs, as the HDD this server is on isn't particularly that astounding - This + already partially generated world, the latter which is uncommon, is probably why I encountered this issue.)

Maybe it's easier to just check async before kicking off the fill task rather than trying to work it in the existing task since it isn't likely it will hit generated chunks after the "initial" spiral out.

(For whatever reason, watchdog doesn't kill my server when it triggers, probably because I set restart-on-crash or w/e to false in spigot.yml(?) and WorldBorder eventually spit out a report of all the already-generated chunks it found after a couple minutes.)

commented

That particular bit of code just reads the 4 KB header of a region file (each region is 32x32 / 1024 chunks) to find out which chunks are already populated, so it can skip loading/generating any existing chunks that have enough adjacent also existing chunks to be fully generated.

Seeing as the actual loading/generating of chunks NEEDS to be done on the main thread using Bukkit's World.loadChunk method, I don't see that it would make sense to just have this one check be split off to its own thread.

Considering what is done (just reading 4 KB from the start of a file for info on each 1024 chunks) I definitely don't understand why it would be hanging there.

commented

Old issue, but not closed, and I guess I found what's the problem here. This code in the run method

if (!forceLoad)
{
	// skip past any chunks which are confirmed as fully generated using our super-special isChunkFullyGenerated routine
	while (worldData.isChunkFullyGenerated(x, z))
	{
		insideBorder = true;
		if (!moveToNext())
			return;
	}
}

tries to find the next un-generated chunk, without comparing current time against start-of-the-loop time. So if the world is big, and mostly populated, this inner loop can take a lot of time to find a chunk that isn't generated.

commented

Hmm, that could be it. I'd think that would take a very very large mostly generated world, since that isChunkFullyGenerated method generally takes a minuscule amount of time, but definitely potential for trouble letting it just run on unchecked there.

I'll see about adding a simple fix for that soon.

commented

8e68e83

Hopefully will take care of it.