mirror of
https://github.com/plexusorg/Plex-FAWE.git
synced 2024-12-23 01:37:37 +00:00
Only implement cache to MaskingExtent when off main thread (#1789)
* Only implement cache to MaskingExtent when off main thread - It's possible for a [Chunk/Char]FilterBlock to be used multiple times in the same "tree" of method calls meaning the mutable paramets (particularly index) get increased within a loop, causing AIOOBs and other issues - This is only possible on the main thread due to the handling of submissions in SingleThreadQueueExtent, as it ensures all operation remains on the main thread to prevent deadlocks - Therefore safe usage of FilterBlocks requires that main-threaded operation create a new [Chunk/Char]FilterBlock instance each time - Further fixes #1681 * Fix typos * Switch to LongFunction
This commit is contained in:
parent
97ab47c90b
commit
b797655d0c
@ -18,6 +18,7 @@ import com.fastasyncworldedit.core.util.collection.CleanableThreadLocal;
|
|||||||
import com.google.common.cache.CacheBuilder;
|
import com.google.common.cache.CacheBuilder;
|
||||||
import com.google.common.cache.CacheLoader;
|
import com.google.common.cache.CacheLoader;
|
||||||
import com.google.common.cache.LoadingCache;
|
import com.google.common.cache.LoadingCache;
|
||||||
|
import com.plotsquared.core.util.AnnotationHelper;
|
||||||
import com.sk89q.jnbt.ByteArrayTag;
|
import com.sk89q.jnbt.ByteArrayTag;
|
||||||
import com.sk89q.jnbt.ByteTag;
|
import com.sk89q.jnbt.ByteTag;
|
||||||
import com.sk89q.jnbt.CompoundTag;
|
import com.sk89q.jnbt.CompoundTag;
|
||||||
@ -54,6 +55,7 @@ import java.util.concurrent.ThreadPoolExecutor;
|
|||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
import java.util.concurrent.atomic.AtomicBoolean;
|
import java.util.concurrent.atomic.AtomicBoolean;
|
||||||
import java.util.function.Function;
|
import java.util.function.Function;
|
||||||
|
import java.util.function.LongFunction;
|
||||||
import java.util.function.Supplier;
|
import java.util.function.Supplier;
|
||||||
|
|
||||||
import static com.google.common.base.Preconditions.checkNotNull;
|
import static com.google.common.base.Preconditions.checkNotNull;
|
||||||
@ -137,6 +139,32 @@ public enum FaweCache implements Trimable {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create a new cache aimed to act as a thread-cache that is safe to the main server thread. If the method is called away
|
||||||
|
* from the main thread, it will return a {@link Function} referencing the {@link LoadingCache} returned by
|
||||||
|
* {@link FaweCache#createCache(Supplier)}. If it is called from the main thread, it will return a {@link Function} that
|
||||||
|
* will always return the result of the given {@link Supplier}. It is designed to prevent issues caused by
|
||||||
|
* internally-mutable and resettable classes such as {@link com.fastasyncworldedit.core.extent.filter.block.CharFilterBlock}
|
||||||
|
* from causing issues when used in edits on the main thread.
|
||||||
|
*
|
||||||
|
* @param withInitial The supplier used to determine the initial value if a thread cache is created, else to provide a new
|
||||||
|
* instance of the class being cached if on the main thread.
|
||||||
|
* @return a {@link Function} referencing a cache, or the given {@link Supplier}
|
||||||
|
* @since TODO
|
||||||
|
*/
|
||||||
|
@AnnotationHelper.ApiDescription(info = "Designed for specific internal use.")
|
||||||
|
public <V> LongFunction<V> createMainThreadSafeCache(Supplier<V> withInitial) {
|
||||||
|
return new LongFunction<>() {
|
||||||
|
private final LoadingCache<Long, V> loadingCache = Fawe.isMainThread() ? null : FaweCache.INSTANCE.createCache(
|
||||||
|
withInitial);
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public V apply(final long input) {
|
||||||
|
return loadingCache != null ? loadingCache.getUnchecked(input) : withInitial.get();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
Exceptions
|
Exceptions
|
||||||
*/
|
*/
|
||||||
|
@ -29,15 +29,13 @@ import com.fastasyncworldedit.core.queue.IBatchProcessor;
|
|||||||
import com.fastasyncworldedit.core.queue.IChunk;
|
import com.fastasyncworldedit.core.queue.IChunk;
|
||||||
import com.fastasyncworldedit.core.queue.IChunkGet;
|
import com.fastasyncworldedit.core.queue.IChunkGet;
|
||||||
import com.fastasyncworldedit.core.queue.IChunkSet;
|
import com.fastasyncworldedit.core.queue.IChunkSet;
|
||||||
import com.google.common.cache.LoadingCache;
|
|
||||||
import com.sk89q.worldedit.WorldEditException;
|
import com.sk89q.worldedit.WorldEditException;
|
||||||
import com.sk89q.worldedit.function.mask.Mask;
|
import com.sk89q.worldedit.function.mask.Mask;
|
||||||
import com.sk89q.worldedit.math.BlockVector3;
|
import com.sk89q.worldedit.math.BlockVector3;
|
||||||
import com.sk89q.worldedit.world.biome.BiomeType;
|
import com.sk89q.worldedit.world.biome.BiomeType;
|
||||||
import com.sk89q.worldedit.world.block.BlockStateHolder;
|
import com.sk89q.worldedit.world.block.BlockStateHolder;
|
||||||
|
|
||||||
import java.util.concurrent.CompletableFuture;
|
import java.util.function.LongFunction;
|
||||||
import java.util.concurrent.Future;
|
|
||||||
|
|
||||||
import static com.google.common.base.Preconditions.checkNotNull;
|
import static com.google.common.base.Preconditions.checkNotNull;
|
||||||
|
|
||||||
@ -46,9 +44,9 @@ import static com.google.common.base.Preconditions.checkNotNull;
|
|||||||
*/
|
*/
|
||||||
public class MaskingExtent extends AbstractDelegateExtent implements IBatchProcessor, Filter {
|
public class MaskingExtent extends AbstractDelegateExtent implements IBatchProcessor, Filter {
|
||||||
|
|
||||||
private Mask mask;
|
|
||||||
//FAWE start
|
//FAWE start
|
||||||
private final LoadingCache<Long, ChunkFilterBlock> threadIdToFilter;
|
private final LongFunction<ChunkFilterBlock> getOrCreateFilterBlock;
|
||||||
|
private Mask mask;
|
||||||
//FAWE end
|
//FAWE end
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -62,16 +60,16 @@ public class MaskingExtent extends AbstractDelegateExtent implements IBatchProce
|
|||||||
checkNotNull(mask);
|
checkNotNull(mask);
|
||||||
this.mask = mask;
|
this.mask = mask;
|
||||||
//FAWE start
|
//FAWE start
|
||||||
this.threadIdToFilter = FaweCache.INSTANCE.createCache(() -> new CharFilterBlock(getExtent()));
|
this.getOrCreateFilterBlock = FaweCache.INSTANCE.createMainThreadSafeCache(() -> new CharFilterBlock(getExtent()));
|
||||||
//FAWE end
|
//FAWE end
|
||||||
}
|
}
|
||||||
|
|
||||||
//FAWE start
|
//FAWE start
|
||||||
private MaskingExtent(Extent extent, Mask mask, LoadingCache<Long, ChunkFilterBlock> threadIdToFilter) {
|
private MaskingExtent(Extent extent, Mask mask, LongFunction<ChunkFilterBlock> getOrCreateFilterBlock) {
|
||||||
super(extent);
|
super(extent);
|
||||||
checkNotNull(mask);
|
checkNotNull(mask);
|
||||||
this.mask = mask;
|
this.mask = mask;
|
||||||
this.threadIdToFilter = threadIdToFilter;
|
this.getOrCreateFilterBlock = getOrCreateFilterBlock;
|
||||||
}
|
}
|
||||||
//FAWE end
|
//FAWE end
|
||||||
|
|
||||||
@ -107,7 +105,7 @@ public class MaskingExtent extends AbstractDelegateExtent implements IBatchProce
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public IChunkSet processSet(final IChunk chunk, final IChunkGet get, final IChunkSet set) {
|
public IChunkSet processSet(final IChunk chunk, final IChunkGet get, final IChunkSet set) {
|
||||||
final ChunkFilterBlock filter = threadIdToFilter.getUnchecked(Thread.currentThread().getId());
|
final ChunkFilterBlock filter = getOrCreateFilterBlock.apply(Thread.currentThread().getId());
|
||||||
return filter.filter(chunk, get, set, MaskingExtent.this);
|
return filter.filter(chunk, get, set, MaskingExtent.this);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -123,12 +121,12 @@ public class MaskingExtent extends AbstractDelegateExtent implements IBatchProce
|
|||||||
if (child == getExtent()) {
|
if (child == getExtent()) {
|
||||||
return this;
|
return this;
|
||||||
}
|
}
|
||||||
return new MaskingExtent(child, this.mask.copy(), this.threadIdToFilter);
|
return new MaskingExtent(child, this.mask.copy(), this.getOrCreateFilterBlock);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Filter fork() {
|
public Filter fork() {
|
||||||
return new MaskingExtent(getExtent(), this.mask.copy(), this.threadIdToFilter);
|
return new MaskingExtent(getExtent(), this.mask.copy(), this.getOrCreateFilterBlock);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
Loading…
Reference in New Issue
Block a user