Skip to content

Commit

Permalink
Fix deadlock in world gen threads on world unload
Browse files Browse the repository at this point in the history
  • Loading branch information
aromaa committed Dec 17, 2023
1 parent 492297b commit c01050f
Show file tree
Hide file tree
Showing 6 changed files with 268 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* This file is part of Sponge, licensed under the MIT License (MIT).
*
* Copyright (c) SpongePowered <https://www.spongepowered.org>
* Copyright (c) contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.spongepowered.common.accessor.world.level.chunk.storage;

import net.minecraft.nbt.CompoundTag;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.gen.Accessor;

@Mixin(targets = "net/minecraft/world/level/chunk/storage/IOWorker$PendingStore")
public interface IOWorker$PendingStoreAccessor {

@Accessor("data") @Nullable CompoundTag accessor$data();
}
1 change: 1 addition & 0 deletions src/accessors/resources/mixins.sponge.accessors.json
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@
"world.level.chunk.LevelChunk$RebindableTickingBlockEntityWrapperAccessor",
"world.level.chunk.LevelChunkAccessor",
"world.level.chunk.storage.ChunkStorageAccessor",
"world.level.chunk.storage.IOWorker$PendingStoreAccessor",
"world.level.dimension.DimensionTypeAccessor",
"world.level.entity.EntityTickListAccessor",
"world.level.entity.PersistentEntitySectionManagerAccessor",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* This file is part of Sponge, licensed under the MIT License (MIT).
*
* Copyright (c) SpongePowered <https://www.spongepowered.org>
* Copyright (c) contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.spongepowered.common.world.level.chunk;

public final class SpongeUnloadedChunkException extends Exception {

public static final SpongeUnloadedChunkException INSTANCE = new SpongeUnloadedChunkException();

private SpongeUnloadedChunkException() {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* This file is part of Sponge, licensed under the MIT License (MIT).
*
* Copyright (c) SpongePowered <https://www.spongepowered.org>
* Copyright (c) contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.spongepowered.common.mixin.core.world.level.chunk;

import com.mojang.datafixers.util.Either;
import net.minecraft.server.level.ChunkHolder;
import net.minecraft.server.level.ServerLevel;
import net.minecraft.server.level.ThreadedLevelLightEngine;
import net.minecraft.server.level.WorldGenRegion;
import net.minecraft.world.level.chunk.ChunkAccess;
import net.minecraft.world.level.chunk.ChunkGenerator;
import net.minecraft.world.level.chunk.ChunkStatus;
import net.minecraft.world.level.chunk.ProtoChunk;
import net.minecraft.world.level.levelgen.blending.Blender;
import net.minecraft.world.level.levelgen.structure.templatesystem.StructureTemplateManager;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Overwrite;
import org.spongepowered.common.world.level.chunk.SpongeUnloadedChunkException;

import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.function.Function;

@Mixin(ChunkStatus.class)
public abstract class ChunkStatusMixin {

/**
* @author aromaa - December 17th, 2023 - 1.19.4
* @reason Fixes a deadlock when the world is unloading/unloaded.
* The Blender#of method calls to the IOWorker#isOldChunkAround which
* submits a task to the main thread while blocking the current thread
* to wait for a response. This fails when the IOWorker has been closed
* as it no longer responds to further work and causes the thread to block
* indefinitely. Fixes this by special casing the IOWorker#isOldChunkAround
* to throw a special exception when the IOWorker has finished up its work
* and catches it here to convert it to a ChunkLoadingFailure.
*
* See IOWorkerMixin#createOldDataForRegion
*/
@Overwrite
private static CompletableFuture<Either<ChunkAccess, ChunkHolder.ChunkLoadingFailure>> lambda$static$6(
final ChunkStatus $$0, final Executor $$1, final ServerLevel $$2, final ChunkGenerator $$3, final StructureTemplateManager $$4,
final ThreadedLevelLightEngine $$5, final Function<ChunkAccess, CompletableFuture<Either<ChunkAccess, ChunkHolder.ChunkLoadingFailure>>> $$6,
final List<ChunkAccess> $$7, final ChunkAccess $$8, final boolean $$9) {
if (!$$9 && $$8.getStatus().isOrAfter($$0)) {
return CompletableFuture.completedFuture(Either.left($$8));
} else {
final WorldGenRegion $$10 = new WorldGenRegion($$2, $$7, $$0, -1);
try { //Sponge: Add try
return $$3.createBiomes($$1, $$2.getChunkSource().randomState(), Blender.of($$10), $$2.structureManager().forWorldGenRegion($$10), $$8)
.thenApply($$1x -> {
if ($$1x instanceof ProtoChunk) {
((ProtoChunk) $$1x).setStatus($$0);
}

return Either.left($$1x);
});
} catch (final Exception e) { //Sponge start: Add catch
if (e.getCause() != SpongeUnloadedChunkException.INSTANCE) {
throw e;
}

return ChunkHolder.UNLOADED_CHUNK_FUTURE;
} //Sponge end
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,60 @@
*/
package org.spongepowered.common.mixin.core.world.level.chunk.storage;

import com.mojang.datafixers.util.Either;
import net.minecraft.nbt.CompoundTag;
import net.minecraft.nbt.IntTag;
import net.minecraft.nbt.Tag;
import net.minecraft.nbt.visitors.CollectFields;
import net.minecraft.nbt.visitors.FieldSelector;
import net.minecraft.resources.ResourceKey;
import net.minecraft.util.thread.ProcessorMailbox;
import net.minecraft.util.thread.StrictQueue;
import net.minecraft.world.level.ChunkPos;
import net.minecraft.world.level.Level;
import net.minecraft.world.level.chunk.storage.IOWorker;
import net.minecraft.world.level.chunk.storage.RegionFileStorage;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.slf4j.Logger;
import org.spongepowered.api.event.SpongeEventFactory;
import org.spongepowered.api.event.world.chunk.ChunkEvent;
import org.spongepowered.asm.mixin.Final;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Overwrite;
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Coerce;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
import org.spongepowered.common.SpongeCommon;
import org.spongepowered.common.accessor.world.level.chunk.storage.IOWorker$PendingStoreAccessor;
import org.spongepowered.common.bridge.world.level.chunk.storage.IOWorkerBridge;
import org.spongepowered.common.event.ShouldFire;
import org.spongepowered.common.event.tracking.PhaseTracker;
import org.spongepowered.common.world.level.chunk.SpongeUnloadedChunkException;
import org.spongepowered.math.vector.Vector3i;

import java.util.BitSet;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;

@Mixin(IOWorker.class)
public abstract class IOWorkerMixin implements IOWorkerBridge {

// @formatter:off
@Shadow @Final private static Logger LOGGER;

@Shadow @Final private AtomicBoolean shutdownRequested;
@Shadow @Final private ProcessorMailbox<StrictQueue.IntRunnable> mailbox;
@Shadow @Final private RegionFileStorage storage;
@Shadow @Final private Map<ChunkPos, IOWorker$PendingStoreAccessor> pendingWrites;

@Shadow protected abstract boolean shadow$isOldChunk(CompoundTag $$0);
@Shadow protected abstract void shadow$tellStorePending();
// @formatter:on

@MonotonicNonNull private ResourceKey<Level> impl$dimension; //We only set this for chunk related IO workers

@Override
Expand All @@ -65,4 +98,78 @@ public abstract class IOWorkerMixin implements IOWorkerBridge {
SpongeCommon.post(postSave);
}
}

/**
* @author aromaa - December 17th, 2023 - 1.19.4
* @reason Fixes a deadlock when the world is unloading/unloaded.
* This method is called from a non server threads and there is a chance
* that the IOWorker is closed before it has the chance to schedule it.
* After the IOWorker has been closed it no longer completes any further
* tasks causing the join to wait indefinitely. Fixes this by submitting
* the task directly instead of indirectly from the background executor
* and not blocking inside of it.
*/
@Overwrite
private CompletableFuture<BitSet> createOldDataForRegion(final int x, final int z) {
//The impl$submitTaskCancellable is related to another fix for a separate deadlock case.
//The returned future is used inside the isOldChunkAround which tries to wait for the
//completion but also ends up deadlocking when the IOWorker closes. This is fixed by
//throwing a special exception when the IOWorker is no longer accepting new tasks.
//See ChunkStatusMixin
return this.impl$submitTaskCancellable(() -> { //Sponge: Use submitTask instead
try {
final ChunkPos min = ChunkPos.minFromRegion(x, z);
final ChunkPos max = ChunkPos.maxFromRegion(x, z);
final BitSet oldDataRegions = new BitSet();
ChunkPos.rangeClosed(min, max).forEach(pos -> {
final CollectFields fields = new CollectFields(new FieldSelector(IntTag.TYPE, "DataVersion"), new FieldSelector(CompoundTag.TYPE, "blending_data"));

try {
//Sponge start: unwrapped the scanChunk content to not submit another task
final IOWorker$PendingStoreAccessor store = this.pendingWrites.get(pos);
if (store != null) {
if (store.accessor$data() != null) {
store.accessor$data().acceptAsRoot(fields);
}
} else {
this.storage.scanChunk(pos, fields);
}
//Sponge end
} catch (final Exception e) {
IOWorkerMixin.LOGGER.warn("Failed to scan chunk {}", pos, e);
return;
}

final Tag result = fields.getResult();
if (result instanceof final CompoundTag compoundTag && this.shadow$isOldChunk(compoundTag)) {
final int regionIndex = pos.getRegionLocalZ() * 32 + pos.getRegionLocalX();
oldDataRegions.set(regionIndex);
}
});
return Either.left(oldDataRegions);
} catch (final Exception e) {
return Either.right(e);
}
});
}

private <T> CompletableFuture<T> impl$submitTaskCancellable(final Supplier<Either<T, Exception>> supplier) {
final CompletableFuture<T> future = this.mailbox.askEither(processor -> new StrictQueue.IntRunnable(0, () -> {
if (!this.shutdownRequested.get()) {
processor.tell(supplier.get());
} else {
processor.tell(Either.right(SpongeUnloadedChunkException.INSTANCE)); //Sponge: Complete exceptionally if shutdown was requested
}

this.shadow$tellStorePending();
}));

//Sponge start: Complete exceptionally if shutdown was requested
if (this.shutdownRequested.get()) {
future.completeExceptionally(SpongeUnloadedChunkException.INSTANCE);
}
//Sponge end

return future;
}
}
1 change: 1 addition & 0 deletions src/mixins/resources/mixins.sponge.core.json
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@
"world.level.block.state.BlockStateMixin",
"world.level.border.WorldBorderMixin",
"world.level.chunk.ChunkGeneratorMixin",
"world.level.chunk.ChunkStatusMixin",
"world.level.chunk.LevelChunkMixin",
"world.level.chunk.storage.IOWorkerMixin",
"world.level.dimension.DimensionTypeMixin",
Expand Down

0 comments on commit c01050f

Please sign in to comment.