Cleanup temporary permission auditing process

common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java

@@ -679,24 +679,30 @@ public abstract class PermissionHolder {
      * @return true if permissions had expired and were removed
      */
     public boolean auditTemporaryPermissions() {
-        Set<Node> removed = new HashSet<>();
-
         // audit temporary nodes first, but don't track ones which are removed
         // we don't call events for transient nodes
-        this.transientNodes.auditTemporaryNodes(null);
+        boolean transientWork = this.transientNodes.auditTemporaryNodes(null);
 
         ImmutableCollection<Node> before = getEnduringNodes().values();
+        Set<Node> removed = new HashSet<>();
 
-        if (!this.enduringNodes.auditTemporaryNodes(removed)) {
+        boolean enduringWork = this.enduringNodes.auditTemporaryNodes(removed);
-            return false;
+        if (enduringWork) {
+            // invalidate
+            invalidateCache();
+
+            // call event
+            ImmutableCollection<Node> after = getEnduringNodes().values();
+            for (Node r : removed) {
+                this.plugin.getEventFactory().handleNodeRemove(r, this, before, after);
+            }
         }
 
-        invalidateCache();
+        if (transientWork && !enduringWork) {
-        ImmutableCollection<Node> after = getEnduringNodes().values();
+            invalidateCache();
-        for (Node r : removed) {
-            this.plugin.getEventFactory().handleNodeRemove(r, this, before, after);
         }
-        return true;
+
+        return transientWork || enduringWork;
     }
 
     private Optional<Node> getAlmostEquals(Node node, NodeMapType type) {

common/src/main/java/me/lucko/luckperms/common/storage/dao/file/ConfigurateDao.java

@@ -394,7 +394,7 @@ public abstract class ConfigurateDao extends AbstractDao {
                     save = true;
                 }
 
-                if (save) {
+                if (save | user.auditTemporaryPermissions()) {
                     saveUser(user);
                 }
             } else {

common/src/main/java/me/lucko/luckperms/common/storage/dao/mongodb/MongoDao.java

@@ -262,7 +262,7 @@ public class MongoDao extends AbstractDao {
                         save = true;
                     }
 
-                    if (save) {
+                    if (save | user.auditTemporaryPermissions()) {
                         c.replaceOne(new Document("_id", user.getUuid()), userToDoc(user));
                     }
                 } else {

common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java

@@ -359,8 +359,8 @@ public class SqlDao extends AbstractDao {
                 Set<Node> nodes = data.stream().map(NodeModel::toNode).collect(Collectors.toSet());
                 user.setEnduringNodes(nodes);
 
-                // Save back to the store if data was changed
+                // Save back to the store if data they were given any defaults or had permissions expire
-                if (this.plugin.getUserManager().giveDefaultIfNeeded(user, false)) {
+                if (this.plugin.getUserManager().giveDefaultIfNeeded(user, false) | user.auditTemporaryPermissions()) {
                     // This should be fine, as the lock will be acquired by the same thread.
                     saveUser(user);
                 }

common/src/main/java/me/lucko/luckperms/common/tasks/ExpireTemporaryTask.java

@@ -26,6 +26,7 @@
 package me.lucko.luckperms.common.tasks;
 
 import me.lucko.luckperms.common.model.Group;
+import me.lucko.luckperms.common.model.PermissionHolder;
 import me.lucko.luckperms.common.model.User;
 import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
 
@@ -40,6 +41,9 @@ public class ExpireTemporaryTask implements Runnable {
     public void run() {
         boolean groupChanges = false;
         for (Group group : this.plugin.getGroupManager().getAll().values()) {
+            if (shouldSkip(group)) {
+                continue;
+            }
             if (group.auditTemporaryPermissions()) {
                 this.plugin.getStorage().saveGroup(group);
                 groupChanges = true;
@@ -47,6 +51,9 @@ public class ExpireTemporaryTask implements Runnable {
         }
 
         for (User user : this.plugin.getUserManager().getAll().values()) {
+            if (shouldSkip(user)) {
+                continue;
+            }
             if (user.auditTemporaryPermissions()) {
                 this.plugin.getStorage().saveUser(user);
                 if (!groupChanges) {
@@ -59,4 +66,18 @@ public class ExpireTemporaryTask implements Runnable {
             this.plugin.getUpdateTaskBuffer().request();
         }
     }
+
+    // return true if the holder's io lock is currently held, false otherwise
+    private static boolean shouldSkip(PermissionHolder holder) {
+        // if the holder is currently being manipulated by the storage dao,
+        // don't attempt to audit temporary permissions
+        if (!holder.getIoLock().tryLock()) {
+            // if #tryLock returns false, it means it's held by something else
+            return true;
+        }
+
+        // immediately release the lock & return false
+        holder.getIoLock().unlock();
+        return false;
+    }
 }