Skip to content

Commit

Permalink
fix: Fix server world lighting providers not stopping all futures whe…
Browse files Browse the repository at this point in the history 
…n the save is exited

Also improved exception logging for threads/`CompletableFuture`s by getting the root cause of thread exceptions and fixed a neighbor count out of range exception in `LightStorage$PropagationFlags#withNeighborCount` by clamping the neighbor count.
  • Loading branch information
@Steveplays28
Steveplays28 committed Aug 1, 2024
1 parent b9e8cf8 commit 606519d
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 19 deletions.
27 changes: 27 additions & 0 deletions ...io/github/steveplays28/noisiumchunkmanager/mixin/world/chunk/light/LightStorageMixin.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.github.steveplays28.noisiumchunkmanager.mixin.world.chunk.light;

import com.llamalad7.mixinextras.sugar.Local;
import com.llamalad7.mixinextras.sugar.ref.LocalIntRef;
import io.github.steveplays28.noisiumchunkmanager.extension.world.chunk.light.LightStorageExtension;
import it.unimi.dsi.fastutil.longs.*;
import it.unimi.dsi.fastutil.objects.ObjectIterator;
Expand All @@ -18,6 +20,7 @@
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.Redirect;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;

import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -371,4 +374,28 @@ public long nextLong() {
public Map<Long, Object> noisiumchunkmanager$getQueuedSections() {
return noisiumchunkmanager$queuedSections;
}

@Mixin(targets = "net.minecraft.world.chunk.light.LightStorage$PropagationFlags")
private static class PropagationFlagsMixin {
@Shadow
@Final
private static int MIN_NEIGHBOR_COUNT;
@Shadow
@Final
private static int MAX_NEIGHBOR_COUNT;
@Shadow
@Final
private static byte NEIGHBOR_COUNT_MASK;

@Inject(method = "withNeighborCount", at = @At(value = "HEAD"), cancellable = true)
private static void noisiumchunkmanager$clampBytes(@NotNull CallbackInfoReturnable<Byte> cir, @Local(argsOnly = true) byte packed, @Local(argsOnly = true) @NotNull LocalIntRef neighborCount) {
if (neighborCount.get() < MIN_NEIGHBOR_COUNT) {
neighborCount.set((byte) MIN_NEIGHBOR_COUNT);
} else if (neighborCount.get() > MAX_NEIGHBOR_COUNT) {
neighborCount.set((byte) MAX_NEIGHBOR_COUNT);
}

cir.setReturnValue((byte) (packed & 0xFFFFFFE0 | neighborCount.get() & NEIGHBOR_COUNT_MASK));
}
}
}
4 changes: 3 additions & 1 deletion ...java/io/github/steveplays28/noisiumchunkmanager/server/world/ServerWorldChunkManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ public ServerWorldChunkManager(@NotNull ServerWorld serverWorld, @NotNull ChunkG
}, threadPoolExecutor).whenComplete((fetchedWorldChunk, throwable) -> {
if (throwable != null) {
NoisiumChunkManager.LOGGER.error(
"Exception thrown while getting a chunk asynchronously:\n{}", ExceptionUtils.getStackTrace(throwable));
"Exception thrown while getting a chunk asynchronously:\n{}",
ExceptionUtils.getStackTrace(ExceptionUtils.getRootCause(throwable))
);
loadingWorldChunks.remove(chunkPos);
return;
}
Expand Down
68 changes: 50 additions & 18 deletions ...b/steveplays28/noisiumchunkmanager/server/world/lighting/ServerWorldLightingProvider.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.github.steveplays28.noisiumchunkmanager.server.world.lighting;

import com.google.common.util.concurrent.ThreadFactoryBuilder;
import dev.architectury.event.events.common.LifecycleEvent;
import dev.architectury.event.events.common.TickEvent;
import io.github.steveplays28.noisiumchunkmanager.NoisiumChunkManager;
import io.github.steveplays28.noisiumchunkmanager.config.NoisiumChunkManagerConfig;
Expand All @@ -22,19 +23,25 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;

import static io.github.steveplays28.noisiumchunkmanager.NoisiumChunkManager.MOD_NAME;

/**
* A lighting provider for {@link ServerWorld}s.
*/
@SuppressWarnings("DataFlowIssue")
public class ServerWorldLightingProvider extends ServerLightingProvider {
private final @NotNull ServerWorld serverWorld;
private final @NotNull Executor threadPoolExecutor;
private final @NotNull List<CompletableFuture<Void>> lightChunkCompletableFutures;

private CompletableFuture<Void> lightUpdatesFuture;
private boolean isStopping;
private CompletableFuture<Void> lightUpdatesCompletableFuture;

public ServerWorldLightingProvider(@NotNull ServerWorld serverWorld) {
super(new ChunkProvider() {
Expand All @@ -60,37 +67,52 @@ public void onLightUpdate(@NotNull LightType lightType, @NotNull ChunkSectionPos
new ThreadFactoryBuilder().setNameFormat(
"Noisium Server World Lighting Provider " + serverWorld.getDimension().effects() + " %d").build()
);
this.lightChunkCompletableFutures = new ArrayList<>();

ServerChunkEvent.WORLD_CHUNK_LOADED.register((instance, worldChunk) -> {
if (!worldChunk.isLightOn()) {
@NotNull var worldChunkPosition = worldChunk.getPos();
instance.getChunkManager().addTicket(ChunkTicketType.LIGHT, worldChunkPosition, 1, worldChunkPosition);
initializeLight(worldChunk, false).thenCompose(chunkWithInitializedLighting ->
light(chunkWithInitializedLighting, false)).whenComplete((litChunk, throwable) -> {
if (throwable != null) {
NoisiumChunkManager.LOGGER.error(
"Exception thrown while lighting a chunk asynchronously:\n{}", ExceptionUtils.getStackTrace(throwable));
return;
}

instance.getChunkManager().removeTicket(ChunkTicketType.LIGHT, worldChunkPosition, 1, worldChunkPosition);
});
if (worldChunk.isLightOn() || instance.getPlayers().isEmpty() || isStopping) {
return;
}

@NotNull var worldChunkPosition = worldChunk.getPos();
// TODO: Change to a functional interface
instance.getChunkManager().addTicket(ChunkTicketType.LIGHT, worldChunkPosition, 2, worldChunkPosition);
initializeLight(worldChunk, false).thenCompose(chunkWithInitializedLighting ->
light(chunkWithInitializedLighting, false)).whenComplete((litChunk, throwable) -> {
if (throwable != null) {
NoisiumChunkManager.LOGGER.error(
"Exception thrown while lighting a chunk asynchronously:\n{}",
ExceptionUtils.getStackTrace(ExceptionUtils.getRootCause(throwable))
);
return;
}

// TODO: Change to a functional interface
instance.getChunkManager().removeTicket(ChunkTicketType.LIGHT, worldChunkPosition, 1, worldChunkPosition);
});
});
ServerChunkEvent.LIGHT_UPDATE.register((instance, lightType, chunkSectionPosition) -> {
if (instance != serverWorld) {
if (instance != serverWorld || isStopping) {
return;
}

onLightUpdateAsync(lightType, chunkSectionPosition);
});
TickEvent.SERVER_LEVEL_POST.register(instance -> {
if (instance != serverWorld) {
if (instance != serverWorld || instance.getPlayers().isEmpty() || isStopping) {
return;
}

tick();
});
LifecycleEvent.SERVER_STOPPING.register(instance -> {
this.isStopping = true;
for (var lightChunkCompletableFuture : lightChunkCompletableFutures) {
lightChunkCompletableFuture.cancel(true);
}

lightChunkCompletableFutures.clear();
});
}

@Override
Expand Down Expand Up @@ -160,6 +182,11 @@ public void enqueueSectionData(@NotNull LightType lightType, @NotNull ChunkSecti

@Override
public @NotNull CompletableFuture<Chunk> initializeLight(@NotNull Chunk chunk, boolean retainLightingData) {
if (isStopping) {
return CompletableFuture.failedFuture(new IllegalStateException(
String.format("Can't initialise chunk lighting because %s Server World Lighting Provider is stopping.", MOD_NAME)));
}

return CompletableFuture.supplyAsync(() -> {
@NotNull var chunkPosition = chunk.getPos();
@NotNull var chunkSections = chunk.getSectionArray();
Expand All @@ -185,6 +212,11 @@ public void enqueueSectionData(@NotNull LightType lightType, @NotNull ChunkSecti

@Override
public @NotNull CompletableFuture<Chunk> light(@NotNull Chunk chunk, boolean excludeBlocks) {
if (isStopping) {
return CompletableFuture.failedFuture(new IllegalStateException(
String.format("Can't light chunk because %s Server World Lighting Provider is stopping.", MOD_NAME)));
}

return CompletableFuture.supplyAsync(() -> {
@NotNull ChunkPos chunkPos = chunk.getPos();
chunk.setLightOn(false);
Expand Down Expand Up @@ -240,11 +272,11 @@ public void tick() {
}

private void doLightUpdatesAsync() {
if (!hasUpdates() || (lightUpdatesFuture != null && !lightUpdatesFuture.isDone())) {
if (!hasUpdates() || (lightUpdatesCompletableFuture != null && !lightUpdatesCompletableFuture.isDone())) {
return;
}

lightUpdatesFuture = CompletableFuture.runAsync(() -> {
lightUpdatesCompletableFuture = CompletableFuture.runAsync(() -> {
blockLightProvider.doLightUpdates();
skyLightProvider.doLightUpdates();
}, threadPoolExecutor);
Expand Down
1 change: 1 addition & 0 deletions common/src/main/resources/noisiumchunkmanager-common.mixins.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"world.chunk.light.BlockLightStorageDataMixin",
"world.chunk.light.ChunkLightProviderMixin",
"world.chunk.light.LightStorageMixin",
"world.chunk.light.LightStorageMixin$PropagationFlagsMixin",
"world.chunk.light.SkyLightStorageMixin",
"world.chunk.light.SkyLightStorageMixin$DataMixin",
"world.gen.feature.OreFeatureMixin",
Expand Down

0 comments on commit 606519d

Please sign in to comment.