From da3fc2e6ead75f38f3c305a8fed491bfca8c150f Mon Sep 17 00:00:00 2001 From: Jordan Date: Fri, 10 Jun 2022 18:50:16 +0100 Subject: [PATCH] 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 --- .../impl/fawe/v1_17_R1_2/PaperweightGetBlocks.java | 7 +++++-- .../fawe/v1_17_R1_2/PaperweightPlatformAdapter.java | 10 ++++++---- .../impl/fawe/v1_18_R1/PaperweightGetBlocks.java | 7 +++++-- .../fawe/v1_18_R1/PaperweightPlatformAdapter.java | 10 ++++++---- .../impl/fawe/v1_18_R2/PaperweightGetBlocks.java | 7 +++++-- .../fawe/v1_18_R2/PaperweightPlatformAdapter.java | 10 ++++++---- .../bukkit/adapter/DelegateSemaphore.java | 11 ++++++++++- 7 files changed, 43 insertions(+), 19 deletions(-) diff --git a/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/PaperweightGetBlocks.java b/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/PaperweightGetBlocks.java index 442631806..b78b1987c 100644 --- a/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/PaperweightGetBlocks.java +++ b/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/PaperweightGetBlocks.java @@ -1,6 +1,7 @@ package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_17_R1_2; import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks; +import com.fastasyncworldedit.bukkit.adapter.DelegateSemaphore; import com.fastasyncworldedit.core.Fawe; import com.fastasyncworldedit.core.FaweCache; 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). - Semaphore lock = PaperweightPlatformAdapter.applyLock(existingSection); PaperweightPlatformAdapter.clearCounts(existingSection); if (PaperLib.isPaper()) { existingSection.tickingList.clear(); } + DelegateSemaphore lock = PaperweightPlatformAdapter.applyLock(existingSection); + // Synchronize to prevent further acquisitions synchronized (lock) { - // lock.acquire(); + lock.acquire(); // Wait until we have the lock + lock.release(); try { sectionLock.writeLock().lock(); if (this.getChunk() != nmsChunk) { diff --git a/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/PaperweightPlatformAdapter.java b/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/PaperweightPlatformAdapter.java index 9ae0da2bd..8d76d9f7f 100644 --- a/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/PaperweightPlatformAdapter.java +++ b/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/PaperweightPlatformAdapter.java @@ -169,9 +169,11 @@ public final class PaperweightPlatformAdapter extends NMSAdapter { return false; } - private static final ThreadLocal SEMAPHORE_THREAD_LOCAL = ThreadLocal.withInitial(() -> new Semaphore(1)); + // There is no point in having a functional semaphore for paper servers. + private static final ThreadLocal SEMAPHORE_THREAD_LOCAL = + ThreadLocal.withInitial(() -> new DelegateSemaphore(1, null)); - static Semaphore applyLock(LevelChunkSection section) { + static DelegateSemaphore applyLock(LevelChunkSection section) { if (PaperLib.isPaper()) { return SEMAPHORE_THREAD_LOCAL.get(); } @@ -180,8 +182,8 @@ public final class PaperweightPlatformAdapter extends NMSAdapter { Unsafe unsafe = ReflectionUtils.getUnsafe(); PalettedContainer blocks = section.getStates(); Semaphore currentLock = (Semaphore) unsafe.getObject(blocks, fieldLockOffset); - if (currentLock instanceof DelegateSemaphore) { - return (DelegateSemaphore) currentLock; + if (currentLock instanceof DelegateSemaphore delegateSemaphore) { + return delegateSemaphore; } DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock); unsafe.putObject(blocks, fieldLockOffset, newLock); diff --git a/worldedit-bukkit/adapters/adapter-1_18/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R1/PaperweightGetBlocks.java b/worldedit-bukkit/adapters/adapter-1_18/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R1/PaperweightGetBlocks.java index b070e5ed1..21e7d44bd 100644 --- a/worldedit-bukkit/adapters/adapter-1_18/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R1/PaperweightGetBlocks.java +++ b/worldedit-bukkit/adapters/adapter-1_18/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R1/PaperweightGetBlocks.java @@ -1,6 +1,7 @@ package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_18_R1; import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks; +import com.fastasyncworldedit.bukkit.adapter.DelegateSemaphore; import com.fastasyncworldedit.core.Fawe; import com.fastasyncworldedit.core.FaweCache; import com.fastasyncworldedit.core.configuration.Settings; @@ -560,10 +561,12 @@ public class PaperweightGetBlocks extends CharGetBlocks implements BukkitGetBloc if (PaperLib.isPaper()) { existingSection.tickingList.clear(); } - Semaphore lock = PaperweightPlatformAdapter.applyLock(existingSection); + DelegateSemaphore lock = PaperweightPlatformAdapter.applyLock(existingSection); + // Synchronize to prevent further acquisitions synchronized (lock) { - // lock.acquire(); + lock.acquire(); // Wait until we have the lock + lock.release(); try { sectionLock.writeLock().lock(); if (this.getChunk() != nmsChunk) { diff --git a/worldedit-bukkit/adapters/adapter-1_18/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R1/PaperweightPlatformAdapter.java b/worldedit-bukkit/adapters/adapter-1_18/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R1/PaperweightPlatformAdapter.java index 29e0bc472..1dc8669c8 100644 --- a/worldedit-bukkit/adapters/adapter-1_18/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R1/PaperweightPlatformAdapter.java +++ b/worldedit-bukkit/adapters/adapter-1_18/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R1/PaperweightPlatformAdapter.java @@ -190,9 +190,11 @@ public final class PaperweightPlatformAdapter extends NMSAdapter { return false; } - private static final ThreadLocal SEMAPHORE_THREAD_LOCAL = ThreadLocal.withInitial(() -> new Semaphore(1)); + // There is no point in having a functional semaphore for paper servers. + private static final ThreadLocal SEMAPHORE_THREAD_LOCAL = + ThreadLocal.withInitial(() -> new DelegateSemaphore(1, null)); - static Semaphore applyLock(LevelChunkSection section) { + static DelegateSemaphore applyLock(LevelChunkSection section) { if (PaperLib.isPaper()) { return SEMAPHORE_THREAD_LOCAL.get(); } @@ -204,8 +206,8 @@ public final class PaperweightPlatformAdapter extends NMSAdapter { fieldThreadingDetectorOffset) ; synchronized(currentThreadingDetector) { Semaphore currentLock = (Semaphore) unsafe.getObject(currentThreadingDetector, fieldLockOffset); - if (currentLock instanceof DelegateSemaphore) { - return (DelegateSemaphore) currentLock; + if (currentLock instanceof DelegateSemaphore delegateSemaphore) { + return delegateSemaphore; } DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock); unsafe.putObject(currentThreadingDetector, fieldLockOffset, newLock); diff --git a/worldedit-bukkit/adapters/adapter-1_18_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R2/PaperweightGetBlocks.java b/worldedit-bukkit/adapters/adapter-1_18_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R2/PaperweightGetBlocks.java index 35d08454f..9f44912cb 100644 --- a/worldedit-bukkit/adapters/adapter-1_18_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R2/PaperweightGetBlocks.java +++ b/worldedit-bukkit/adapters/adapter-1_18_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R2/PaperweightGetBlocks.java @@ -1,6 +1,7 @@ package com.sk89q.worldedit.bukkit.adapter.impl.fawe.v1_18_R2; import com.fastasyncworldedit.bukkit.adapter.BukkitGetBlocks; +import com.fastasyncworldedit.bukkit.adapter.DelegateSemaphore; import com.fastasyncworldedit.core.Fawe; import com.fastasyncworldedit.core.FaweCache; import com.fastasyncworldedit.core.configuration.Settings; @@ -576,10 +577,12 @@ public class PaperweightGetBlocks extends CharGetBlocks implements BukkitGetBloc if (PaperLib.isPaper()) { existingSection.tickingList.clear(); } - Semaphore lock = PaperweightPlatformAdapter.applyLock(existingSection); + DelegateSemaphore lock = PaperweightPlatformAdapter.applyLock(existingSection); + // Synchronize to prevent further acquisitions synchronized (lock) { - // lock.acquire(); + lock.acquire(); // Wait until we have the lock + lock.release(); try { sectionLock.writeLock().lock(); if (this.getChunk() != nmsChunk) { diff --git a/worldedit-bukkit/adapters/adapter-1_18_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R2/PaperweightPlatformAdapter.java b/worldedit-bukkit/adapters/adapter-1_18_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R2/PaperweightPlatformAdapter.java index 4c18fa15d..8316ae008 100644 --- a/worldedit-bukkit/adapters/adapter-1_18_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R2/PaperweightPlatformAdapter.java +++ b/worldedit-bukkit/adapters/adapter-1_18_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_18_R2/PaperweightPlatformAdapter.java @@ -189,9 +189,11 @@ public final class PaperweightPlatformAdapter extends NMSAdapter { return false; } - private static final ThreadLocal SEMAPHORE_THREAD_LOCAL = ThreadLocal.withInitial(() -> new Semaphore(1)); + // There is no point in having a functional semaphore for paper servers. + private static final ThreadLocal SEMAPHORE_THREAD_LOCAL = + ThreadLocal.withInitial(() -> new DelegateSemaphore(1, null)); - static Semaphore applyLock(LevelChunkSection section) { + static DelegateSemaphore applyLock(LevelChunkSection section) { if (PaperLib.isPaper()) { return SEMAPHORE_THREAD_LOCAL.get(); } @@ -205,8 +207,8 @@ public final class PaperweightPlatformAdapter extends NMSAdapter { ); synchronized (currentThreadingDetector) { Semaphore currentLock = (Semaphore) unsafe.getObject(currentThreadingDetector, fieldLockOffset); - if (currentLock instanceof DelegateSemaphore) { - return currentLock; + if (currentLock instanceof DelegateSemaphore delegateSemaphore) { + return delegateSemaphore; } DelegateSemaphore newLock = new DelegateSemaphore(1, currentLock); unsafe.putObject(currentThreadingDetector, fieldLockOffset, newLock); diff --git a/worldedit-bukkit/src/main/java/com/fastasyncworldedit/bukkit/adapter/DelegateSemaphore.java b/worldedit-bukkit/src/main/java/com/fastasyncworldedit/bukkit/adapter/DelegateSemaphore.java index 87cdf2d1d..c31a98b5f 100644 --- a/worldedit-bukkit/src/main/java/com/fastasyncworldedit/bukkit/adapter/DelegateSemaphore.java +++ b/worldedit-bukkit/src/main/java/com/fastasyncworldedit/bukkit/adapter/DelegateSemaphore.java @@ -14,6 +14,9 @@ public class DelegateSemaphore extends Semaphore { // this is bad @Override public synchronized boolean tryAcquire() { + if (delegate == null) { + return true; + } try { this.delegate.acquire(); return true; @@ -24,11 +27,17 @@ public class DelegateSemaphore extends Semaphore { @Override public synchronized void acquire() throws InterruptedException { + if (delegate == null) { + return; + } this.delegate.acquire(); } @Override - public synchronized void release() { + public void release() { + if (delegate == null) { + return; + } this.delegate.release(); }