reduce the amount of synchronisation/locks being used unneecessarily and add nullcheck

worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPermissionAttachmentManager.java

@@ -1,58 +1,44 @@
 package com.sk89q.worldedit.bukkit;
 
+import javax.annotation.Nullable;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 import org.bukkit.entity.Player;
 import org.bukkit.permissions.PermissionAttachment;
 
 public class BukkitPermissionAttachmentManager {
 
     private final WorldEditPlugin plugin;
-    private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
-    private final Map<Player, PermissionAttachment> attachments = new HashMap();
+    private final Map<Player, PermissionAttachment> attachments = new HashMap<>();
 
     public BukkitPermissionAttachmentManager(WorldEditPlugin plugin) {
         this.plugin = plugin;
     }
 
-    public synchronized PermissionAttachment addAttachment(Player p) {
-        PermissionAttachment attach;
+    public PermissionAttachment getOrAddAttachment(@Nullable final Player p) {
+        if (p == null) {
+            return null;
+        }
+        PermissionAttachment attachment = attachments.get(p);
 
-        //try find attachment
-        lock.readLock().lock();
-        try {
-            attach = attachments.get(p);
-        } finally {
-            lock.readLock().unlock();
+        if (attachment != null) {
+            return attachment;
         }
 
-        if (attach == null) {
-            lock.writeLock().lock();
-            try {
-                //check again - do not remove this
-                attach = attachments.get(p);
-                if (attach == null) {
-                    attach = p.addAttachment(plugin);
-                    attachments.put(p, attach);
-                }
-            } finally {
-                lock.writeLock().unlock();
-            }
+        synchronized (this) {
+            return attachments.computeIfAbsent(p, k -> k.addAttachment(plugin));
         }
-        return attach;
     }
 
-    public void removeAttachment(Player p) {
-        PermissionAttachment attach;
-        lock.writeLock().lock();
-        try {
-            attach = attachments.remove(p);
-        } finally {
-            lock.writeLock().unlock();
+    public void removeAttachment(@Nullable final Player p) {
+        if (p == null || attachments.get(p) == null) {
+            return;
         }
-        if (attach != null) {
-            p.removeAttachment(attach);
+        synchronized (this) {
+            PermissionAttachment attach = attachments.remove(p);
+            if (attach != null) {
+                p.removeAttachment(attach);
+            }
         }
     }
 }

worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPlayer.java

@@ -67,7 +67,6 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Locale;
 import java.util.UUID;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.annotation.Nullable;
 
@@ -81,13 +80,13 @@ public class BukkitPlayer extends AbstractPlayerActor {
         super(getExistingMap(WorldEditPlugin.getInstance(), player));
         this.plugin = WorldEditPlugin.getInstance();
         this.player = player;
-        this.permAttachment = plugin.getPermissionAttachmentManager().addAttachment(player);
+        this.permAttachment = plugin.getPermissionAttachmentManager().getOrAddAttachment(player);
     }
 
     public BukkitPlayer(WorldEditPlugin plugin, Player player) {
         this.plugin = plugin;
         this.player = player;
-        this.permAttachment = plugin.getPermissionAttachmentManager().addAttachment(player);
+        this.permAttachment = plugin.getPermissionAttachmentManager().getOrAddAttachment(player);
         if (Settings.IMP.CLIPBOARD.USE_DISK) {
             loadClipboardFromDisk();
         }