diff --git a/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java b/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java index 27c64a8b..45bbc9db 100644 --- a/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java +++ b/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 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 before = getEnduringNodes().values(); + Set removed = new HashSet<>(); - if (!this.enduringNodes.auditTemporaryNodes(removed)) { - return false; + boolean enduringWork = this.enduringNodes.auditTemporaryNodes(removed); + if (enduringWork) { + // invalidate + invalidateCache(); + + // call event + ImmutableCollection after = getEnduringNodes().values(); + for (Node r : removed) { + this.plugin.getEventFactory().handleNodeRemove(r, this, before, after); + } } - invalidateCache(); - ImmutableCollection after = getEnduringNodes().values(); - for (Node r : removed) { - this.plugin.getEventFactory().handleNodeRemove(r, this, before, after); + if (transientWork && !enduringWork) { + invalidateCache(); } - return true; + + return transientWork || enduringWork; } private Optional getAlmostEquals(Node node, NodeMapType type) { diff --git a/common/src/main/java/me/lucko/luckperms/common/storage/dao/file/ConfigurateDao.java b/common/src/main/java/me/lucko/luckperms/common/storage/dao/file/ConfigurateDao.java index 2b957dfb..d2c71031 100644 --- a/common/src/main/java/me/lucko/luckperms/common/storage/dao/file/ConfigurateDao.java +++ b/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 { diff --git a/common/src/main/java/me/lucko/luckperms/common/storage/dao/mongodb/MongoDao.java b/common/src/main/java/me/lucko/luckperms/common/storage/dao/mongodb/MongoDao.java index 15a8d919..cbb773af 100644 --- a/common/src/main/java/me/lucko/luckperms/common/storage/dao/mongodb/MongoDao.java +++ b/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 { diff --git a/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java b/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java index c028e926..58efa1e8 100644 --- a/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java +++ b/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java @@ -359,8 +359,8 @@ public class SqlDao extends AbstractDao { Set nodes = data.stream().map(NodeModel::toNode).collect(Collectors.toSet()); user.setEnduringNodes(nodes); - // Save back to the store if data was changed - if (this.plugin.getUserManager().giveDefaultIfNeeded(user, false)) { + // Save back to the store if data they were given any defaults or had permissions expire + 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); } diff --git a/common/src/main/java/me/lucko/luckperms/common/tasks/ExpireTemporaryTask.java b/common/src/main/java/me/lucko/luckperms/common/tasks/ExpireTemporaryTask.java index 74d4300c..85ebf797 100644 --- a/common/src/main/java/me/lucko/luckperms/common/tasks/ExpireTemporaryTask.java +++ b/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; + } } \ No newline at end of file