Keep timed permission expiry on platform threads #338
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.ScheduledExecutorService; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| import org.bukkit.Bukkit; | ||
|
|
@@ -49,9 +50,12 @@ public class PermissionHandler { | |
|
|
||
| @Getter | ||
| private final ScheduledExecutorService timer = Executors.newScheduledThreadPool(1); | ||
| private final AtomicBoolean acceptingExpirations = new AtomicBoolean(true); | ||
| private final Object[] stateLocks = new Object[64]; | ||
|
|
||
| public PermissionHandler(AdvancedCorePlugin plugin) { | ||
| this.plugin = plugin; | ||
| for (int i = 0; i < stateLocks.length; i++) stateLocks[i] = new Object(); | ||
|
|
||
| // Restore timed permissions from previous shutdown (stored as expireAtMillis) | ||
| if (plugin.getServerDataFile().getData() != null | ||
|
|
@@ -88,6 +92,63 @@ public PermissionHandler(AdvancedCorePlugin plugin) { | |
| } | ||
| } | ||
|
|
||
|
|
||
| void scheduleExpiration(PlayerPermissionHandler handle, String permission, long expectedExpireAt, long delayMillis) { | ||
| if (!acceptingExpirations.get()) return; | ||
| try { | ||
| timer.schedule(() -> dispatchExpiration(handle, permission, expectedExpireAt), | ||
| Math.max(0L, delayMillis), java.util.concurrent.TimeUnit.MILLISECONDS); | ||
| } catch (RuntimeException failure) { | ||
| plugin.debug(failure); | ||
| throw failure; | ||
| } | ||
| } | ||
|
|
||
| void dispatchExpiration(PlayerPermissionHandler handle, String permission, long expectedExpireAt) { | ||
| if (!acceptingExpirations.get() || !handle.isExpirationCurrent(permission, expectedExpireAt)) return; | ||
| try { | ||
| plugin.getBukkitScheduler().runTask(plugin, () -> { | ||
| if (!acceptingExpirations.get() || !handle.isExpirationCurrent(permission, expectedExpireAt)) return; | ||
| Player player = Bukkit.getPlayer(handle.getUuid()); | ||
| if (player == null) { | ||
| expireOfflineOrRetry(handle, permission, expectedExpireAt); | ||
| return; | ||
| } | ||
| try { | ||
| plugin.getBukkitScheduler().runTask(plugin, () -> { | ||
| if (!acceptingExpirations.get()) return; | ||
| handle.expirePermission(permission, expectedExpireAt, true); | ||
| }, player); | ||
| } catch (RuntimeException failure) { | ||
| plugin.debug(failure); | ||
| retryExpiration(handle, permission, expectedExpireAt); | ||
| } | ||
|
Comment on lines
+122
to
+125
There was a problem hiding this comment.
If the entity-owner AGENTS.md reference: AGENTS.md:L57-L58 Useful? React with 👍 / 👎. |
||
| }); | ||
| } catch (RuntimeException failure) { | ||
| plugin.debug(failure); | ||
| retryExpiration(handle, permission, expectedExpireAt); | ||
| } | ||
| } | ||
|
|
||
| private void expireOfflineOrRetry(PlayerPermissionHandler handle, String permission, long expectedExpireAt) { | ||
| boolean active; | ||
| UUID uuid = handle.getUuid(); | ||
| synchronized (stateLock(uuid)) { | ||
| if (permsToAdd.get(uuid) == handle) { | ||
| handle.expirePermission(permission, expectedExpireAt, false); | ||
| return; | ||
| } | ||
| active = perms.get(uuid) == handle; | ||
| } | ||
| if (active) retryExpiration(handle, permission, expectedExpireAt); | ||
| } | ||
|
|
||
| private void retryExpiration(PlayerPermissionHandler handle, String permission, long expectedExpireAt) { | ||
| if (!acceptingExpirations.get() || !handle.isExpirationCurrent(permission, expectedExpireAt)) return; | ||
| try { scheduleExpiration(handle, permission, expectedExpireAt, 1_000L); } | ||
| catch (RuntimeException ignored) { /* scheduleExpiration already reported the rejection */ } | ||
| } | ||
|
|
||
| public void addPermission(Player player, String permission) { | ||
| addPermission(player.getUniqueId(), permission); | ||
| } | ||
|
|
@@ -115,22 +176,32 @@ public void addPermission(UUID uuid, String permission) { | |
| return; | ||
| } | ||
|
|
||
| synchronized (stateLock(uuid)) { | ||
| for (String perm : permission.split(Pattern.quote("|"))) { | ||
| PlayerPermissionHandler handle = perms.get(uuid); | ||
|
|
||
| if (handle != null) { | ||
| handle.addPerm(perm); | ||
| continue; | ||
| } | ||
| PlayerPermissionHandler pending = permsToAdd.get(uuid); | ||
| if (pending != null) { | ||
| pending.addOfflinePerm(perm, ParsedDuration.empty()); | ||
| continue; | ||
| } | ||
|
|
||
| Player p = Bukkit.getPlayer(uuid); | ||
| if (p != null) { | ||
| PermissionAttachment attachment = p.addAttachment(plugin); | ||
| PlayerPermissionHandler newHandle = new PlayerPermissionHandler(uuid, attachment, this).addPerm(perm); | ||
| perms.put(uuid, newHandle); | ||
| } else { | ||
| permsToAdd.compute(uuid, (ignored, existing) -> { | ||
| PlayerPermissionHandler target = existing == null | ||
| ? new PlayerPermissionHandler(uuid, null, this) : existing; | ||
| return target.addOfflinePerm(perm, ParsedDuration.empty()); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -152,22 +223,33 @@ public void addPermission(UUID uuid, String permission, ParsedDuration duration) | |
| return; | ||
| } | ||
|
|
||
| synchronized (stateLock(uuid)) { | ||
| for (String perm : permission.split(Pattern.quote("|"))) { | ||
| PlayerPermissionHandler handle = perms.get(uuid); | ||
|
|
||
| if (handle != null) { | ||
| handle.addExpiration(perm, duration); | ||
| continue; | ||
| } | ||
| PlayerPermissionHandler pending = permsToAdd.get(uuid); | ||
| if (pending != null) { | ||
| pending.addOfflinePerm(perm, duration); | ||
| continue; | ||
| } | ||
|
|
||
| Player p = Bukkit.getPlayer(uuid); | ||
| if (p != null) { | ||
| PermissionAttachment attachment = p.addAttachment(plugin); | ||
| PlayerPermissionHandler newHandle = new PlayerPermissionHandler(uuid, attachment, this) | ||
| .addExpiration(perm, duration); | ||
| perms.put(uuid, newHandle); | ||
| } else { | ||
| permsToAdd.compute(uuid, (ignored, existing) -> { | ||
| PlayerPermissionHandler target = existing == null | ||
| ? new PlayerPermissionHandler(uuid, null, this) : existing; | ||
| return target.addOfflinePerm(perm, duration); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -188,19 +270,20 @@ public void addPermission(UUID uuid, String permission, long seconds) { | |
| */ | ||
| public void login(Player player) { | ||
| UUID uuid = player.getUniqueId(); | ||
| synchronized (stateLock(uuid)) { | ||
| PlayerPermissionHandler handle = perms.get(uuid); | ||
| if (handle != null) { | ||
| handle.setAttachment(player.addAttachment(plugin)); | ||
| handle.onLogin(player); | ||
| return; | ||
| } | ||
|
|
||
| PlayerPermissionHandler pending = permsToAdd.remove(uuid); | ||
| if (pending != null) { | ||
| pending.setAttachment(player.addAttachment(plugin)); | ||
| pending.onLogin(player); | ||
| perms.put(uuid, pending); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -213,26 +296,46 @@ public void login(Player player) { | |
| * </p> | ||
| */ | ||
| public void logout(Player player) { | ||
| UUID uuid = player.getUniqueId(); | ||
| synchronized (stateLock(uuid)) { | ||
| PlayerPermissionHandler handle = perms.remove(uuid); | ||
| if (handle == null) return; | ||
|
|
||
| try { | ||
| if (handle.getAttachment() != null) player.removeAttachment(handle.getAttachment()); | ||
| } catch (Throwable ignored) { | ||
| } | ||
|
|
||
| handle.setAttachment(null); | ||
| handle.onLogout(player); | ||
| permsToAdd.merge(uuid, handle, (pending, moved) -> { | ||
| java.util.Map<String, Long> queued = pending.offlinePermissionSnapshot(); | ||
| moved.mergeOfflinePermissions(queued); | ||
| return moved; | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| public void removePermission(UUID uuid) { | ||
| synchronized (stateLock(uuid)) { | ||
| perms.remove(uuid); | ||
| permsToAdd.remove(uuid); | ||
| } | ||
| } | ||
|
|
||
| void removePermission(UUID uuid, PlayerPermissionHandler expected) { | ||
| synchronized (stateLock(uuid)) { | ||
| perms.remove(uuid, expected); | ||
| permsToAdd.remove(uuid, expected); | ||
| } | ||
| } | ||
|
|
||
| void removePermissionIfEmpty(UUID uuid, PlayerPermissionHandler expected, boolean attachmentIsOffline) { | ||
| synchronized (stateLock(uuid)) { | ||
| if (!expected.isHandlerEmpty(attachmentIsOffline)) return; | ||
| perms.remove(uuid, expected); | ||
| permsToAdd.remove(uuid, expected); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -251,45 +354,48 @@ public void removePermission(UUID uuid, String playerName, String permission) { | |
| return; | ||
| } | ||
|
|
||
| synchronized (stateLock(uuid)) { | ||
| PlayerPermissionHandler handle = perms.get(uuid); | ||
| if (handle == null) handle = permsToAdd.get(uuid); | ||
| if (handle == null) return; | ||
|
|
||
| for (String perm : permission.split(Pattern.quote("|"))) { | ||
| handle.removePermission(perm); | ||
| if (playerName != null && !playerName.isEmpty()) { | ||
| plugin.debug("Removing temp permission " + perm + " from " + playerName); | ||
| } else { | ||
| plugin.debug("Removing temp permission " + perm + " from " + uuid); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private Object stateLock(UUID uuid) { | ||
| return stateLocks[(uuid.hashCode() & Integer.MAX_VALUE) % stateLocks.length]; | ||
| } | ||
|
|
||
| /** | ||
| * Persists timed permissions for both online + offline handlers. | ||
| */ | ||
| public void shutDown() { | ||
| // Fence every timer/global/entity callback before taking persistence snapshots. | ||
| // A callback already inside a handler monitor finishes before timedPermissionSnapshot(). | ||
| acceptingExpirations.set(false); | ||
| timer.shutdownNow(); | ||
| saveTimedPerms(perms); | ||
| saveTimedPerms(permsToAdd); | ||
| plugin.getServerDataFile().saveData(); | ||
| } | ||
|
|
||
| private void saveTimedPerms(ConcurrentHashMap<UUID, PlayerPermissionHandler> map) { | ||
| for (PlayerPermissionHandler handle : map.values()) { | ||
| java.util.Map<String, Long> snapshot = handle.timedPermissionSnapshot(); | ||
| if (snapshot.isEmpty()) continue; | ||
|
|
||
| ArrayList<String> list = new ArrayList<>(); | ||
| for (Entry<String, Long> entry : snapshot.entrySet()) { | ||
| list.add(entry.getKey() + "%line%" + entry.getValue()); | ||
| } | ||
| plugin.getServerDataFile().getData().set("TimedPermissions." + handle.getUuid(), list); | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the wall clock moves backward after scheduling, the executor's relative delay can elapse while the absolute
expectedExpireAtis still in the future, but this check validates only ownership and the later callback immediately removes the grant. The fresh evidence relative to the earlier wall-clock comment is that the final code contains no wall-clock guard in eitherdispatchExpirationorexpirePermission, so the current failure is premature revocation rather than a skipped expiration. Recompute the remaining wall-clock duration and reschedule whenever it is positive.Useful? React with 👍 / 👎.