Fix DelegateSemaphore synchronisation issues on Spigot (#1781)

* Fix DelegateSemaphore synchronisation issues on Spigot
 - Also effectively nullify it on paper - the synchronisation on the object is enough

* Remove unneeded imports
This commit is contained in:
Jordan 2022-06-10 18:50:16 +01:00 committed by GitHub
parent 7f8ce69563
commit da3fc2e6ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 43 additions and 19 deletions

View File

@ -1,6 +1,7 @@
package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_17_R1_2; package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_17_R1_2;
import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks; import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks;
import com.fastasyncworldedit.bukkit.adapter.DelegateSemaphore;
import com.fastasyncworldedit.core.Fawe; import com.fastasyncworldedit.core.Fawe;
import com.fastasyncworldedit.core.FaweCache; import com.fastasyncworldedit.core.FaweCache;
import com.fastasyncworldedit.core.configuration.Settings; import com.fastasyncworldedit.core.configuration.Settings;
@ -505,14 +506,16 @@ public class PaperweightGetBlocks extends CharGetBlocks implements BukkitGetBloc
} }
//ensure that the server doesn't try to tick the chunksection while we're editing it (again). //ensure that the server doesn't try to tick the chunksection while we're editing it (again).
Semaphore lock = PaperweightPlatformAdapter.applyLock(existingSection);
PaperweightPlatformAdapter.clearCounts(existingSection); PaperweightPlatformAdapter.clearCounts(existingSection);
if (PaperLib.isPaper()) { if (PaperLib.isPaper()) {
existingSection.tickingList.clear(); existingSection.tickingList.clear();
} }
DelegateSemaphore lock = PaperweightPlatformAdapter.applyLock(existingSection);
// Synchronize to prevent further acquisitions
synchronized (lock) { synchronized (lock) {
// lock.acquire(); lock.acquire(); // Wait until we have the lock
lock.release();
try { try {
sectionLock.writeLock().lock(); sectionLock.writeLock().lock();
if (this.getChunk() != nmsChunk) { if (this.getChunk() != nmsChunk) {

View File

@ -169,9 +169,11 @@ public final class PaperweightPlatformAdapter extends NMSAdapter {
return false; return false;
} }
private static final ThreadLocal<Semaphore> SEMAPHORE_THREAD_LOCAL = ThreadLocal.withInitial(() -> new Semaphore(1)); // There is no point in having a functional semaphore for paper servers.
private static final ThreadLocal<DelegateSemaphore> SEMAPHORE_THREAD_LOCAL =
ThreadLocal.withInitial(() -> new DelegateSemaphore(1, null));
static Semaphore applyLock(LevelChunkSection section) { static DelegateSemaphore applyLock(LevelChunkSection section) {
if (PaperLib.isPaper()) { if (PaperLib.isPaper()) {
return SEMAPHORE_THREAD_LOCAL.get(); return SEMAPHORE_THREAD_LOCAL.get();
} }
@ -180,8 +182,8 @@ public final class PaperweightPlatformAdapter extends NMSAdapter {
Unsafe unsafe = ReflectionUtils.getUnsafe(); Unsafe unsafe = ReflectionUtils.getUnsafe();
PalettedContainer<net.minecraft.world.level.block.state.BlockState> blocks = section.getStates(); PalettedContainer<net.minecraft.world.level.block.state.BlockState> blocks = section.getStates();
Semaphore currentLock = (Semaphore) unsafe.getObject(blocks, fieldLockOffset); Semaphore currentLock = (Semaphore) unsafe.getObject(blocks, fieldLockOffset);
if (currentLock instanceof DelegateSemaphore) { if (currentLock instanceof DelegateSemaphore delegateSemaphore) {
return (DelegateSemaphore) currentLock; return delegateSemaphore;
} }
DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock); DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock);
unsafe.putObject(blocks, fieldLockOffset, newLock); unsafe.putObject(blocks, fieldLockOffset, newLock);

View File

@ -1,6 +1,7 @@
package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_18_R1; package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_18_R1;
import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks; import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks;
import com.fastasyncworldedit.bukkit.adapter.DelegateSemaphore;
import com.fastasyncworldedit.core.Fawe; import com.fastasyncworldedit.core.Fawe;
import com.fastasyncworldedit.core.FaweCache; import com.fastasyncworldedit.core.FaweCache;
import com.fastasyncworldedit.core.configuration.Settings; import com.fastasyncworldedit.core.configuration.Settings;
@ -560,10 +561,12 @@ public class PaperweightGetBlocks extends CharGetBlocks implements BukkitGetBloc
if (PaperLib.isPaper()) { if (PaperLib.isPaper()) {
existingSection.tickingList.clear(); existingSection.tickingList.clear();
} }
Semaphore lock = PaperweightPlatformAdapter.applyLock(existingSection); DelegateSemaphore lock = PaperweightPlatformAdapter.applyLock(existingSection);
// Synchronize to prevent further acquisitions
synchronized (lock) { synchronized (lock) {
// lock.acquire(); lock.acquire(); // Wait until we have the lock
lock.release();
try { try {
sectionLock.writeLock().lock(); sectionLock.writeLock().lock();
if (this.getChunk() != nmsChunk) { if (this.getChunk() != nmsChunk) {

View File

@ -190,9 +190,11 @@ public final class PaperweightPlatformAdapter extends NMSAdapter {
return false; return false;
} }
private static final ThreadLocal<Semaphore> SEMAPHORE_THREAD_LOCAL = ThreadLocal.withInitial(() -> new Semaphore(1)); // There is no point in having a functional semaphore for paper servers.
private static final ThreadLocal<DelegateSemaphore> SEMAPHORE_THREAD_LOCAL =
ThreadLocal.withInitial(() -> new DelegateSemaphore(1, null));
static Semaphore applyLock(LevelChunkSection section) { static DelegateSemaphore applyLock(LevelChunkSection section) {
if (PaperLib.isPaper()) { if (PaperLib.isPaper()) {
return SEMAPHORE_THREAD_LOCAL.get(); return SEMAPHORE_THREAD_LOCAL.get();
} }
@ -204,8 +206,8 @@ public final class PaperweightPlatformAdapter extends NMSAdapter {
fieldThreadingDetectorOffset) ; fieldThreadingDetectorOffset) ;
synchronized(currentThreadingDetector) { synchronized(currentThreadingDetector) {
Semaphore currentLock = (Semaphore) unsafe.getObject(currentThreadingDetector, fieldLockOffset); Semaphore currentLock = (Semaphore) unsafe.getObject(currentThreadingDetector, fieldLockOffset);
if (currentLock instanceof DelegateSemaphore) { if (currentLock instanceof DelegateSemaphore delegateSemaphore) {
return (DelegateSemaphore) currentLock; return delegateSemaphore;
} }
DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock); DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock);
unsafe.putObject(currentThreadingDetector, fieldLockOffset, newLock); unsafe.putObject(currentThreadingDetector, fieldLockOffset, newLock);

View File

@ -1,6 +1,7 @@
package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_18_R2; package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_18_R2;
import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks; import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks;
import com.fastasyncworldedit.bukkit.adapter.DelegateSemaphore;
import com.fastasyncworldedit.core.Fawe; import com.fastasyncworldedit.core.Fawe;
import com.fastasyncworldedit.core.FaweCache; import com.fastasyncworldedit.core.FaweCache;
import com.fastasyncworldedit.core.configuration.Settings; import com.fastasyncworldedit.core.configuration.Settings;
@ -576,10 +577,12 @@ public class PaperweightGetBlocks extends CharGetBlocks implements BukkitGetBloc
if (PaperLib.isPaper()) { if (PaperLib.isPaper()) {
existingSection.tickingList.clear(); existingSection.tickingList.clear();
} }
Semaphore lock = PaperweightPlatformAdapter.applyLock(existingSection); DelegateSemaphore lock = PaperweightPlatformAdapter.applyLock(existingSection);
// Synchronize to prevent further acquisitions
synchronized (lock) { synchronized (lock) {
// lock.acquire(); lock.acquire(); // Wait until we have the lock
lock.release();
try { try {
sectionLock.writeLock().lock(); sectionLock.writeLock().lock();
if (this.getChunk() != nmsChunk) { if (this.getChunk() != nmsChunk) {

View File

@ -189,9 +189,11 @@ public final class PaperweightPlatformAdapter extends NMSAdapter {
return false; return false;
} }
private static final ThreadLocal<Semaphore> SEMAPHORE_THREAD_LOCAL = ThreadLocal.withInitial(() -> new Semaphore(1)); // There is no point in having a functional semaphore for paper servers.
private static final ThreadLocal<DelegateSemaphore> SEMAPHORE_THREAD_LOCAL =
ThreadLocal.withInitial(() -> new DelegateSemaphore(1, null));
static Semaphore applyLock(LevelChunkSection section) { static DelegateSemaphore applyLock(LevelChunkSection section) {
if (PaperLib.isPaper()) { if (PaperLib.isPaper()) {
return SEMAPHORE_THREAD_LOCAL.get(); return SEMAPHORE_THREAD_LOCAL.get();
} }
@ -205,8 +207,8 @@ public final class PaperweightPlatformAdapter extends NMSAdapter {
); );
synchronized (currentThreadingDetector) { synchronized (currentThreadingDetector) {
Semaphore currentLock = (Semaphore) unsafe.getObject(currentThreadingDetector, fieldLockOffset); Semaphore currentLock = (Semaphore) unsafe.getObject(currentThreadingDetector, fieldLockOffset);
if (currentLock instanceof DelegateSemaphore) { if (currentLock instanceof DelegateSemaphore delegateSemaphore) {
return currentLock; return delegateSemaphore;
} }
DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock); DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock);
unsafe.putObject(currentThreadingDetector, fieldLockOffset, newLock); unsafe.putObject(currentThreadingDetector, fieldLockOffset, newLock);

View File

@ -14,6 +14,9 @@ public class DelegateSemaphore extends Semaphore {
// this is bad // this is bad
@Override @Override
public synchronized boolean tryAcquire() { public synchronized boolean tryAcquire() {
if (delegate == null) {
return true;
}
try { try {
this.delegate.acquire(); this.delegate.acquire();
return true; return true;
@ -24,11 +27,17 @@ public class DelegateSemaphore extends Semaphore {
@Override @Override
public synchronized void acquire() throws InterruptedException { public synchronized void acquire() throws InterruptedException {
if (delegate == null) {
return;
}
this.delegate.acquire(); this.delegate.acquire();
} }
@Override @Override
public synchronized void release() { public void release() {
if (delegate == null) {
return;
}
this.delegate.release(); this.delegate.release();
} }