Skip to content

Commit

Permalink
fix: do not clash ChunkHolder recycling with processors that extend o…
Browse files Browse the repository at this point in the history
…utside the chunk (#2353)
  • Loading branch information
dordsor21 authored Jul 20, 2023
1 parent fe1859e commit 0a8a479
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 143 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

import com.sk89q.jnbt.CompoundTag;
import com.sk89q.jnbt.DoubleTag;
import com.sk89q.jnbt.IntArrayTag;
import com.sk89q.jnbt.ListTag;
import com.sk89q.jnbt.LongTag;
import com.sk89q.jnbt.NBTUtils;
import com.sk89q.jnbt.StringTag;
import com.sk89q.jnbt.Tag;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,6 @@ public SingleThreadQueueExtent(int minY, int maxY) {
this.maxY = maxY;
}

/**
* Safety check to ensure that the thread being used matches the one being initialized on. - Can
* be removed later
*/
private void checkThread() {
if (Thread.currentThread() != currentThread && currentThread != null) {
throw new UnsupportedOperationException(
"This class must be used from a single thread. Use multiple queues for concurrent operations");
}
}

@Override
public void enableQueue() {
enabledQueue = true;
Expand Down Expand Up @@ -154,10 +143,10 @@ protected synchronized void reset() {
return;
}
if (!this.chunks.isEmpty()) {
getChunkLock.lock();
for (IChunk chunk : this.chunks.values()) {
chunk.recycle();
}
getChunkLock.lock();
this.chunks.clear();
getChunkLock.unlock();
}
Expand Down Expand Up @@ -233,9 +222,21 @@ public <V extends Future<V>> V submit(IQueueChunk chunk) {
*/
private <V extends Future<V>> V submitUnchecked(IQueueChunk chunk) {
if (chunk.isEmpty()) {
chunk.recycle();
Future result = Futures.immediateFuture(null);
return (V) result;
if (chunk instanceof ChunkHolder<?> holder) {
long age = holder.initAge();
// Ensure we've given time for the chunk to be used - it was likely used for a reason!
if (age < 5) {
try {
Thread.sleep(5 - age);
} catch (InterruptedException ignored) {
}
}
}
if (chunk.isEmpty()) {
chunk.recycle();
Future result = Futures.immediateFuture(null);
return (V) result;
}
}

if (Fawe.isMainThread()) {
Expand Down Expand Up @@ -451,6 +452,7 @@ private void iterateSubmissions() {
@Override
public synchronized void flush() {
if (!chunks.isEmpty()) {
getChunkLock.lock();
if (MemUtil.isMemoryLimited()) {
for (IQueueChunk chunk : chunks.values()) {
final Future future = submitUnchecked(chunk);
Expand All @@ -467,7 +469,6 @@ public synchronized void flush() {
}
}
}
getChunkLock.lock();
chunks.clear();
getChunkLock.unlock();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.fastasyncworldedit.core.queue.implementation.chunk;

import com.fastasyncworldedit.core.FaweCache;
import com.fastasyncworldedit.core.concurrent.ReentrantWrappedStampedLock;
import com.fastasyncworldedit.core.configuration.Settings;
import com.fastasyncworldedit.core.extent.filter.block.ChunkFilterBlock;
import com.fastasyncworldedit.core.extent.processor.EmptyBatchProcessor;
Expand All @@ -26,6 +25,8 @@
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Future;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

/**
* An abstract {@link IChunk} class that implements basic get/set blocks.
Expand All @@ -43,7 +44,7 @@ public static ChunkHolder newInstance() {
return POOL.poll();
}

private final ReentrantWrappedStampedLock calledLock = new ReentrantWrappedStampedLock();
private final Lock calledLock = new ReentrantLock();

private volatile IChunkGet chunkExisting; // The existing chunk (e.g. a clipboard, or the world, before changes)
private volatile IChunkSet chunkSet; // The blocks to be set to the chunkExisting
Expand All @@ -55,6 +56,7 @@ public static ChunkHolder newInstance() {
private int bitMask = -1; // Allow forceful setting of bitmask (for lighting)
private boolean isInit = false; // Lighting handles queue differently. It relies on the chunk cache and not doing init.
private boolean createCopy = false;
private long initTime = -1L;

private ChunkHolder() {
this.delegate = NULL;
Expand All @@ -66,6 +68,7 @@ public void init(IBlockDelegate delegate) {

@Override
public synchronized void recycle() {
calledLock.lock();
delegate = NULL;
if (chunkSet != null) {
chunkSet.recycle();
Expand All @@ -74,6 +77,11 @@ public synchronized void recycle() {
chunkExisting = null;
extent = null;
POOL.offer(this);
calledLock.unlock();
}

public long initAge() {
return System.currentTimeMillis() - initTime;
}

public synchronized IBlockDelegate getDelegate() {
Expand All @@ -84,10 +92,10 @@ public synchronized IBlockDelegate getDelegate() {
* If the chunk is currently being "called", this method will block until completed.
*/
private void checkAndWaitOnCalledLock() {
if (calledLock.isLocked()) {
if (!calledLock.tryLock()) {
calledLock.lock();
calledLock.unlock();
}
calledLock.unlock();
}

@Override
Expand Down Expand Up @@ -1024,6 +1032,7 @@ private synchronized IChunkGet newWrappedGet() {

@Override
public synchronized <V extends IChunk> void init(IQueueExtent<V> extent, int chunkX, int chunkZ) {
this.initTime = System.currentTimeMillis();
this.extent = extent;
this.chunkX = chunkX;
this.chunkZ = chunkZ;
Expand All @@ -1040,14 +1049,15 @@ public synchronized <V extends IChunk> void init(IQueueExtent<V> extent, int chu
@Override
public synchronized T call() {
calledLock.lock();
final long stamp = calledLock.getStampChecked();
if (chunkSet != null && !chunkSet.isEmpty()) {
this.delegate = GET;
chunkSet.setBitMask(bitMask);
try {
IChunkSet copy = chunkSet.createCopy();
chunkSet = null;
return this.call(copy, () -> calledLock.unlock(stamp));
return this.call(copy, () -> {
// Do nothing
});
} catch (Throwable t) {
calledLock.unlock();
throw t;
Expand All @@ -1072,6 +1082,7 @@ public synchronized T call(IChunkSet set, Runnable finalize) {
} else {
finalizer = finalize;
}
calledLock.unlock();
return get.call(set, finalizer);
}
return null;
Expand Down

0 comments on commit 0a8a479

Please sign in to comment.