From 65747bf8f8d4e42ab7c05e4c045e7183f96fd9ac Mon Sep 17 00:00:00 2001 From: dordsor21 Date: Fri, 25 Sep 2020 15:02:09 +0100 Subject: [PATCH] accessing clipboards should not be synchronized to LocalSession (#653) * accessing clipboards should not be synchronized to LocalSession I believe this may be the issue causing thread locks when wrapping new players. If we're attempting to run a synchronised method within the LocalSesison when initialising it, it may go wrong..? * nullcheck within synchronisation --- .../com/sk89q/worldedit/LocalSession.java | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/LocalSession.java b/worldedit-core/src/main/java/com/sk89q/worldedit/LocalSession.java index 069c45bd2..477e446e6 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/LocalSession.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/LocalSession.java @@ -134,6 +134,7 @@ public class LocalSession implements TextureHolder { }); private transient volatile Integer historyNegativeIndex; private transient ClipboardHolder clipboard; + private transient final Object clipboardLock = new Object(); private transient boolean superPickaxe = false; private transient BlockTool pickaxeMode = new SinglePickaxe(); private transient final Int2ObjectOpenHashMap tools = new Int2ObjectOpenHashMap<>(0); @@ -767,19 +768,26 @@ public class LocalSession implements TextureHolder { * @return clipboard * @throws EmptyClipboardException thrown if no clipboard is set */ - public synchronized ClipboardHolder getClipboard() throws EmptyClipboardException { - if (clipboard == null) { - throw new EmptyClipboardException(); + public ClipboardHolder getClipboard() throws EmptyClipboardException { + synchronized (clipboardLock) { + if (clipboard == null) { + throw new EmptyClipboardException(); + } + return clipboard; } - return clipboard; } @Nullable - public synchronized ClipboardHolder getExistingClipboard() { - return clipboard; + public ClipboardHolder getExistingClipboard() { + synchronized (clipboardLock) { + if (clipboard == null) { + return null; + } + return clipboard; + } } - public synchronized void addClipboard(@Nonnull MultiClipboardHolder toAppend) { + public void addClipboard(@Nonnull MultiClipboardHolder toAppend) { checkNotNull(toAppend); ClipboardHolder existing = getExistingClipboard(); MultiClipboardHolder multi; @@ -804,15 +812,18 @@ public class LocalSession implements TextureHolder { * * @param clipboard the clipboard, or null if the clipboard is to be cleared */ - public synchronized void setClipboard(@Nullable ClipboardHolder clipboard) { - if (this.clipboard == clipboard) return; + public void setClipboard(@Nullable ClipboardHolder clipboard) { + synchronized (clipboardLock) { + if (this.clipboard == clipboard) + return; - if (this.clipboard != null) { - if (clipboard == null || !clipboard.contains(this.clipboard.getClipboard())) { - this.clipboard.close(); + if (this.clipboard != null) { + if (clipboard == null || !clipboard.contains(this.clipboard.getClipboard())) { + this.clipboard.close(); + } } + this.clipboard = clipboard; } - this.clipboard = clipboard; } /**