Several fixes to actual, probable and possible synchronocity issues (#691)

* Several fixes to actual, probable and possible synchronocity issues
 - Ensure that all edits are queued onto the same AsyncNotifyQueue by actually delegating to the parent player in PlayerProxy
 - Ensure that the order editsessions are being remembered on the LocalSession is being respected by using a fair ReentrentLock
 - Ensure a chunk cannot be called when an update is being called

* Don't add locks to GetBlocks
This commit is contained in:
dordsor21 2020-10-08 21:15:05 +01:00 committed by GitHub
parent 4407749219
commit 5b97c0abcd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 111 additions and 98 deletions

View File

@ -18,7 +18,6 @@ import com.boydti.fawe.bukkit.regions.TownyFeature;
import com.boydti.fawe.bukkit.regions.Worldguard; import com.boydti.fawe.bukkit.regions.Worldguard;
import com.boydti.fawe.bukkit.util.BukkitTaskMan; import com.boydti.fawe.bukkit.util.BukkitTaskMan;
import com.boydti.fawe.bukkit.util.ItemUtil; import com.boydti.fawe.bukkit.util.ItemUtil;
import com.boydti.fawe.bukkit.util.VaultUtil;
import com.boydti.fawe.bukkit.util.image.BukkitImageViewer; import com.boydti.fawe.bukkit.util.image.BukkitImageViewer;
import com.boydti.fawe.config.Settings; import com.boydti.fawe.config.Settings;
import com.boydti.fawe.regions.FaweMaskManager; import com.boydti.fawe.regions.FaweMaskManager;

View File

@ -106,7 +106,7 @@ public class BukkitGetBlocks_1_15_2_Copy extends BukkitGetBlocks_1_15_2 {
} }
protected void storeSection(int layer) { protected void storeSection(int layer) {
update(layer, blocks[layer]); blocks[layer] = update(layer, null).clone();
} }
@Override @Override

View File

@ -107,7 +107,7 @@ public class BukkitGetBlocks_1_16_1_Copy extends BukkitGetBlocks_1_16_1 {
} }
protected void storeSection(int layer) { protected void storeSection(int layer) {
update(layer, blocks[layer]); blocks[layer] = update(layer, null).clone();
} }
@Override @Override

View File

@ -107,7 +107,7 @@ public class BukkitGetBlocks_1_16_2_Copy extends BukkitGetBlocks_1_16_2 {
} }
protected void storeSection(int layer) { protected void storeSection(int layer) {
update(layer, blocks[layer]); blocks[layer] = update(layer, null).clone();
} }
@Override @Override

View File

@ -92,9 +92,12 @@ import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.ListIterator; import java.util.ListIterator;
import java.util.Map; import java.util.Map;
import java.util.Random;
import java.util.TimeZone; import java.util.TimeZone;
import java.util.UUID; import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import javax.annotation.Nonnull; import javax.annotation.Nonnull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
@ -116,7 +119,7 @@ public class LocalSession implements TextureHolder {
// Session related // Session related
private transient RegionSelector selector = new CuboidRegionSelector(); private transient RegionSelector selector = new CuboidRegionSelector();
private transient boolean placeAtPos1 = false; private transient boolean placeAtPos1 = false;
private transient List<Object> history = Collections.synchronizedList(new LinkedList<Object>() { private final transient List<Object> history = Collections.synchronizedList(new LinkedList<Object>() {
@Override @Override
public Object get(int index) { public Object get(int index) {
Object value = super.get(index); Object value = super.get(index);
@ -135,6 +138,7 @@ public class LocalSession implements TextureHolder {
private transient volatile Integer historyNegativeIndex; private transient volatile Integer historyNegativeIndex;
private transient ClipboardHolder clipboard; private transient ClipboardHolder clipboard;
private transient final Object clipboardLock = new Object(); private transient final Object clipboardLock = new Object();
private transient final Lock historyWriteLock = new ReentrantLock(true);
private transient boolean superPickaxe = false; private transient boolean superPickaxe = false;
private transient BlockTool pickaxeMode = new SinglePickaxe(); private transient BlockTool pickaxeMode = new SinglePickaxe();
private final transient Int2ObjectOpenHashMap<Tool> tools = new Int2ObjectOpenHashMap<>(0); private final transient Int2ObjectOpenHashMap<Tool> tools = new Int2ObjectOpenHashMap<>(0);
@ -439,109 +443,113 @@ public class LocalSession implements TextureHolder {
return null; return null;
} }
public synchronized void remember(Identifiable player, World world, ChangeSet changeSet, FaweLimit limit) { public void remember(Identifiable player, World world, ChangeSet changeSet, FaweLimit limit) {
if (Settings.IMP.HISTORY.USE_DISK) { historyWriteLock.lock();
LocalSession.MAX_HISTORY_SIZE = Integer.MAX_VALUE; try {
} if (Settings.IMP.HISTORY.USE_DISK) {
if (changeSet.size() == 0) { LocalSession.MAX_HISTORY_SIZE = Integer.MAX_VALUE;
return; }
} if (changeSet.size() == 0) {
loadSessionHistoryFromDisk(player.getUniqueId(), world); return;
if (changeSet instanceof ChangeSet) { }
ListIterator<Object> iter = history.listIterator(); loadSessionHistoryFromDisk(player.getUniqueId(), world);
int i = 0; if (changeSet instanceof ChangeSet) {
int cutoffIndex = history.size() - getHistoryNegativeIndex(); ListIterator<Object> iter = history.listIterator();
while (iter.hasNext()) { int i = 0;
Object item = iter.next(); int cutoffIndex = history.size() - getHistoryNegativeIndex();
if (++i > cutoffIndex) { while (iter.hasNext()) {
ChangeSet oldChangeSet; Object item = iter.next();
if (item instanceof ChangeSet) { if (++i > cutoffIndex) {
oldChangeSet = (ChangeSet) item; ChangeSet oldChangeSet;
} else { if (item instanceof ChangeSet) {
oldChangeSet = getChangeSet(item); oldChangeSet = (ChangeSet) item;
} else {
oldChangeSet = getChangeSet(item);
}
historySize -= MainUtil.getSize(oldChangeSet);
iter.remove();
} }
historySize -= MainUtil.getSize(oldChangeSet);
iter.remove();
} }
} }
historySize += MainUtil.getSize(changeSet);
history.add(changeSet);
if (getHistoryNegativeIndex() != 0) {
setDirty();
historyNegativeIndex = 0;
}
if (limit != null) {
int limitMb = limit.MAX_HISTORY;
while (((!Settings.IMP.HISTORY.USE_DISK && history.size() > MAX_HISTORY_SIZE) || (historySize >> 20) > limitMb) && history.size() > 1) {
ChangeSet item = (ChangeSet) history.remove(0);
item.delete();
long size = MainUtil.getSize(item);
historySize -= size;
}
}
} finally {
historyWriteLock.unlock();
} }
historySize += MainUtil.getSize(changeSet); }
history.add(changeSet);
if (getHistoryNegativeIndex() != 0) { public void remember(EditSession editSession, boolean append, int limitMb) {
setDirty(); historyWriteLock.lock();
historyNegativeIndex = 0; try {
} if (Settings.IMP.HISTORY.USE_DISK) {
if (limit != null) { LocalSession.MAX_HISTORY_SIZE = Integer.MAX_VALUE;
int limitMb = limit.MAX_HISTORY; }
// It should have already been flushed, but just in case!
editSession.flushQueue();
if (editSession.getChangeSet() == null || limitMb == 0 || historySize >> 20 > limitMb && !append) {
return;
}
ChangeSet changeSet = editSession.getChangeSet();
if (changeSet.isEmpty()) {
return;
}
Player player = editSession.getPlayer();
if (player != null) {
loadSessionHistoryFromDisk(player.getUniqueId(), editSession.getWorld());
}
// Destroy any sessions after this undo point
if (append) {
ListIterator<Object> iter = history.listIterator();
int i = 0;
int cutoffIndex = history.size() - getHistoryNegativeIndex();
while (iter.hasNext()) {
Object item = iter.next();
if (++i > cutoffIndex) {
ChangeSet oldChangeSet;
if (item instanceof ChangeSet) {
oldChangeSet = (ChangeSet) item;
} else {
oldChangeSet = getChangeSet(item);
}
historySize -= MainUtil.getSize(oldChangeSet);
iter.remove();
}
}
}
historySize += MainUtil.getSize(changeSet);
if (append) {
history.add(changeSet);
if (getHistoryNegativeIndex() != 0) {
setDirty();
historyNegativeIndex = 0;
}
} else {
history.add(0, changeSet);
}
while (((!Settings.IMP.HISTORY.USE_DISK && history.size() > MAX_HISTORY_SIZE) || (historySize >> 20) > limitMb) && history.size() > 1) { while (((!Settings.IMP.HISTORY.USE_DISK && history.size() > MAX_HISTORY_SIZE) || (historySize >> 20) > limitMb) && history.size() > 1) {
ChangeSet item = (ChangeSet) history.remove(0); ChangeSet item = (ChangeSet) history.remove(0);
item.delete(); item.delete();
long size = MainUtil.getSize(item); long size = MainUtil.getSize(item);
historySize -= size; historySize -= size;
} }
} } finally {
} historyWriteLock.unlock();
public synchronized void remember(EditSession editSession, boolean append, int limitMb) {
if (Settings.IMP.HISTORY.USE_DISK) {
LocalSession.MAX_HISTORY_SIZE = Integer.MAX_VALUE;
}
// It should have already been flushed, but just in case!
editSession.flushQueue();
if (editSession.getChangeSet() == null || limitMb == 0 || historySize >> 20 > limitMb && !append) {
return;
}
/*
// Don't store anything if no changes were made
if (editSession.size() == 0) {
return;
}
*/
ChangeSet changeSet = editSession.getChangeSet();
if (changeSet.isEmpty()) {
return;
}
Player player = editSession.getPlayer();
if (player != null) {
loadSessionHistoryFromDisk(player.getUniqueId(), editSession.getWorld());
}
// Destroy any sessions after this undo point
if (append) {
ListIterator<Object> iter = history.listIterator();
int i = 0;
int cutoffIndex = history.size() - getHistoryNegativeIndex();
while (iter.hasNext()) {
Object item = iter.next();
if (++i > cutoffIndex) {
ChangeSet oldChangeSet;
if (item instanceof ChangeSet) {
oldChangeSet = (ChangeSet) item;
} else {
oldChangeSet = getChangeSet(item);
}
historySize -= MainUtil.getSize(oldChangeSet);
iter.remove();
}
}
}
historySize += MainUtil.getSize(changeSet);
if (append) {
history.add(changeSet);
if (getHistoryNegativeIndex() != 0) {
setDirty();
historyNegativeIndex = 0;
}
} else {
history.add(0, changeSet);
}
while (((!Settings.IMP.HISTORY.USE_DISK && history.size() > MAX_HISTORY_SIZE) || (historySize >> 20) > limitMb) && history.size() > 1) {
ChangeSet item = (ChangeSet) history.remove(0);
item.delete();
long size = MainUtil.getSize(item);
historySize -= size;
} }
} }

View File

@ -684,6 +684,7 @@ public abstract class AbstractPlayerActor implements Actor, Player, Cloneable {
* @param async TODO description * @param async TODO description
* @return false if the task was ran or queued * @return false if the task was ran or queued
*/ */
@Override
public boolean runAction(Runnable ifFree, boolean checkFree, boolean async) { public boolean runAction(Runnable ifFree, boolean checkFree, boolean async) {
if (checkFree) { if (checkFree) {
if (runningCount.get() != 0) { if (runningCount.get() != 0) {

View File

@ -83,6 +83,11 @@ public class PlayerProxy extends AbstractPlayerActor {
return basePlayer.getBlockInHand(handSide); return basePlayer.getBlockInHand(handSide);
} }
@Override
public boolean runAction(Runnable ifFree, boolean checkFree, boolean async) {
return basePlayer.runAction(ifFree, checkFree, async);
}
@Override @Override
public UUID getUniqueId() { public UUID getUniqueId() {
return basePlayer.getUniqueId(); return basePlayer.getUniqueId();