There are two codepaths that invoke saving of chunks.
1. ThreadedAnvilChunkStorage.tryUnloadChunk(...) which saves chunks that are no longer kept alive by any chunk ticket, before finally unloading them from the world.
2. ThreadedAnvilChunkStorage.save(flush) which saves chunks periodically and upon closing the world.
net.minecraft.server.world.ThreadedAnvilChunkStorage.java
protected void save(boolean flush) {
if (flush) {
...
} else {
this.chunkHolders.values().stream().filter(ChunkHolder::isAccessible).forEach((chunkHolder) -> {
Chunk chunk = (Chunk)chunkHolder.getSavingFuture().getNow((Object)null);
if (chunk instanceof ReadOnlyChunk || chunk instanceof WorldChunk) {
this.save(chunk);
chunkHolder.updateAccessibleStatus();
}
});
}
}This code saves all accessible chunks, i.e., chunks that were in the BORDER status at some point since the last save, periodically (when flush is false) and upon closing the world (when flush is true). The second case is similar (and hence not included in the code snippet) but additionally waits for pending futures in the accessible chunks.
The point is now that neither of these cases covers chunks that are still kept alive by chunk tickets when closing the world, but do not have a sufficient ticket level to be BORDER chunks. Those are chunks that are only partially generated, but can already contain features and light propagations from adjacent chunks. Not saving these chunks will hence lead to corrupted features and broken light at chunk borders.
In the following I will present some concrete examples where chunk tickets can keep such chunks alive and will hence prevent saving. Some other ticket types can most certainly lead to the same effect.
1. Spawn chunks
The first example uses START tickets created on spawn chunks (the same works with FORCED tickets), which are not removed upon unloading the world and hence will keep some chunks alive.
Create a new world with render distance 2 (e.g., with seed
-4530634556500121041)Quit and reload directly after generating the world
One can now turn up the render distance again
Observe that features between the 11th and 12th chunk away from the spawn point are broken (World spawn loads chunks in a radius of 11 into
[media]BORDERstatus)
Due to the low render distance, chunks in distance 12 from the spawn point are not requested into BORDER status, but still kept from unloading due to the START ticket, hence triggering the described issue.
2. Overload the lighting engine
The second example uses LIGHT tickets created during worldgen to keep chunks alive. While these tickets will eventually be removed when the worldgen is finished, the goal here is to overload the lighting engine so that these generations steps will not be finished (and hence the tickets will not be removed) in time before the flush save completes. Note that the flush save only waits for accessible chunks to finish any remaining futures, but not for partially generated chunks.
Note that this behavior is quite random and hence the following steps might require several attempts for reproduction or may not work at all, depending on the setup. Nevertheless, I think this example is still of practical relevance as described in the final section.
In order to stress the lighting engine we can try the following steps
Create an
AMPLIFIEDworld with render distance 32Fly a few chunks in one direction and then close the world immediately
Repeat the previous step several times, always flying in the same direction
After repeating for a few times, revisit the generated chunks again and see if some corrupted features or light glitches occur.
[media](Note that there are also some lighting glitches on the chunk border)
Closing the world immediately after flying a few chunks does not give the lighting engine enough time to finish its jobs before unloading the world (at least sometimes), hence causing the described issue.
3. Player tickets
As pointed out by @unknown, PLAYER tickets can cause this issue very deterministically (comparable to the START tickets) and are hence probably the most common and severe case. While PLAYER tickets are explicitly removed upon shutting down the world by disconnecting all players before saving, this ticket removal does involve some threading and hence might not be completed before the saving executes, hence keeping all chunks loaded by players alive during saving and consequently triggering the issue.
4. Interacting with the world
Every world interaction, or more precisely every call to World.getChunk(...), creates an UNKNOWN chunk ticket with the intent to keep the chunk loaded and hence valid until the end of the tick. Every ticking entity and every player creates such a ticket at its location every tick, e.g., due to movement operations querying the chunk. These UNKNOWN tickets are in fact not cleared at the end of the current tick, but at the start of the next one. Consequently, these UNKNOWN tickets from the last tick are not purged upon shutting down the world and are hence still present during saving, prohibiting unloading and saving of some chunks (in contrast, PLAYER tickets are explicitly removed upon shutting down the world by disconnecting all players).
This does not seem to be an issue on high render distances, presumably because there are no entities far away from the player, at least not in the very last tick, and hence no such UNKNOWN tickets are created away from the player, whereas the PLAYER ticket itself has a much higher range and hence dominates these UNKNOWN tickets. Hence the UNKNOWN tickets only affect chunks that were already accessible due to the PLAYER ticket and hence do not trigger the saving issue.
However, this argument does not work for small render distances, e.g., render distance 2, where the UNKNOWN tickets do keep chunks alive that were not yet accessible, hence causing the saving issues. Indeed, the issue does show up deterministically when exploring new chunks with a render distance of 2 and reloading the world every few chunks.
Remarks
I think the issue described here is a good candidate for explaining MC-125007 (respectively MC-188086).
I am not sure, though, to which extent it is related to MC-214793. It again seems to be a good explanation for the corrupted features that seem to occur in conjunction with the dark chunks. However, it fails to explain why these chunks are completely dark; it only explains missing light propagations at the chunk borders, since these are discarded upon saving. So, it looks like some other unknown issue is interfering here as well.
Related issues
is duplicated by
relates to
Attachments
Comments
migrated
marcono1234
Can confirm for 1.16.5 and 21w17a, but for 1.16.5 I only tested the first case "1. Spawn chunks".
migrated
Chunks appear to be generated+decorated twice on exit/reload, leading to cut off resources, have also been able to confirm via debugging, see: https://github.com/MinecraftForge/MinecraftForge/issues/7766#issuecomment-841545896. Great job tracking this down, hope it can be fixed soon.
migrated
After generating a (rectangular) spawn area and exiting/rejoining the SP client, then exploring to generate chunks, a band of chunks 1 chunk wide around the previously generated area is generated a second time.
This does not happen only around spawn, it repeats on every exit/rejoin. Using the steps in the aforementioned Forge issue, after adding some logic to make regenerated chunks visible, I generated spawn, exited/rejoined, traveled north, found a 1-wide band of regenerated chunks, exited/rejoined again, traveled farther north, found another band of regenerated chunks, repeated with same result, etc. To be clear: Chunks appear to have both chunkgen and decoration run twice, so this doesn't only affect resources spawned by adjacent chunks.
*Note: Tested with MC 1.16.5 Forge 36.1.6 SP client render distance 16, though the problem appears identical to the vanilla issue. Hope it helps, cheers.
PhiPro
@@unknown I added a third example to the original report which should explain your case. As I said, the list is certianly not exhaustive.
I guess you did your tests with a render distance of 2? Just out of interest, does it also happen on higher render distances, e.g., 20? (away from spawn to exclude problem 1)
migrated
Have done some debugging with Forge client, trying to see what happens to the affected band of chunks on the boundaries of the generated area when exiting.
net.minecraft.world.server.saveAllChunks() (different mappings than original poster, otherwise the same code).
if (flush)
{ ... }
else {
this.visibleChunkMap.values().stream().filter(ChunkHolder::wasAccessibleSinceLastSave).forEach((chunkHolder) -> {
IChunk ichunk = chunkHolder.getChunkToSave().getNow((IChunk)null);
if (ichunk instanceof ChunkPrimerWrapper || ichunk instanceof Chunk)
{ this.save(ichunk); chunkHolder.refreshAccessibility(); }
});
}When exiting the world, saveAllChunks is called, it reaches "IChunk ichunk = chunkHolder.getChunkToSave().getNow((IChunk)null);", however ichunk is null after this. When inspecting chunkHolder for these chunks, I see the following:
*Edit: After more debugging, and disabling the wasAccessibleSinceLastSave filter, the affected chunks turned out to be of type ChunkPrimer, which also makes them fail the instanceof check, present in both the flush and non-flush branches.
Unfortunately I'm new to 1.16 coming from 1.12, so I'm not knowledgeable on these things, will continue to do some digging though. If you need any more info let me know, hope it helps, cheers.
All tests done with render distance 16.
migrated
@Philipp Provenzano: All tests were done with render distance 16, just did another test with 32, when traveling north after each exit/rejoin, I get a band of regenerated chunks every time. Have confirmed findings via code and without mods, though I also have an (admittedly goofy) test setup to make the problem clearly visible:
[media]
*North at 10 o clock, though lines appear no matter which direction you travel.
PhiPro
Well, removal of the PLAYER tickets involves some threading, so it's likely that they actually don't get removed entirely before saving (so this effect is probably slightly random). So I would guess that this is actually caused by the PLAYER tickets that are not cleared before saving. Anyway, the general issue stays the same: chunks not getting saved due to not being unloaded because of some tickets.
I'll maybe add this case to the original report later (it's probably the most dominant case).
migrated
Taking a look at solving this with some modders, @Philipp Provenzano if you're interested join us on the Minecraft WorldGen discord at https://discord.gg/BuBGds9, dev-1-17 channel. A few more things to note:
When you generate a new world with view range 32, stand still, let chunks finish spawning, reduce view range to 16, then exit/rejoin, there does not appear to be a problem.
When you generate a new world with view range 16, stand still, let chunks finish spawning, exit/rejoin, then increase view range to 32, you get a perfect 1 chunk wide rectangular band of regenerated chunks. (this is also mentioned in original post)
This makes it easy to debug at least.
migrated
Possible workaround / clue:
Generate a world with view range 32, join and let chunks finish spawning, reduce view range to 2, then exit, set view range back to 32 in options menu, then rejoin, explore in any direction. The problem is gone, around spawn as well as when exploring further and on consecutive exit/rejoins. As long as view range is reduced before exit, the problem does not occur.
PhiPro
@@unknown There is already an easy hackfix by SuperCoder79, simply removing the isAccessible filters: https://github.com/SuperCoder7979/chunksavingfix-fabric
Note though that a proper fix should redesign some aspects of the saving code, in particular because there are some other race conditions (caused by removing chunks from the chunkHolder map before saving them, so that the save(...) method does not necessarily wait for those chunks while they are neither in the chunkHolder map nor in the unloadQueue) which aren't covered by this fix.
Nevertheless, such a redesign is probably out of scope for mods and should be done by Mojang. The simple fix is enough for the deterministic part of the issue and should be good enough as hackfix for the moment.
migrated
@Philipp Provenzano, SuperCoder is with us on the discord, unfortunately the hackfix did not work.
SuperCoder79
Hello, with a little bit more debugging it can be seen that the line ".filter(chunk-> chunk instanceof ImposterProtoChunk || chunk instanceof LevelChunk)" is also causing problems, as it's filtering out ProtoChunks along that border. By adding "chunk instanceof ProtoChunk" to the filter, it will include ProtoChunks and fix the issue. Like stated above, this is definitely a stop-gap fix and a better fix should probably be implemented to fix the problem at it's root cause instead of it's side effects.
ampolive
Can confirm in 21w40a.
ampolive
Can confirm in 21w41a. Relates to MC-185253.
ampolive
I have tried to reproduce this three times in 22w12a using the first example, but I was unsuccessful. However, this is very likely still an issue.
isXander
Tried about 8 times with no results in 1.18.2
isXander
Nevermind. Actually saw this happening in my normal survival server on accident.
[media]
Lentern
I didn't see this report when I made MC-252059, so you're welcome to look at that for another example.
rkullenius
I am unable to reproduce this issue, can you confirm that you are still able to reproduce this issue on the latest version?