Skip to content

Commit

Permalink
Flag handler atomicity fix - Java booleans are inexplicably not atomic
Browse files Browse the repository at this point in the history
also verbose debugs
  • Loading branch information
mcmonkey4eva committed Apr 21, 2022
1 parent fb6c8b2 commit 41ed061
Showing 1 changed file with 28 additions and 17 deletions.
Expand Up @@ -22,6 +22,7 @@
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;

public class PlayerFlagHandler implements Listener {

Expand All @@ -35,9 +36,7 @@ public static class CachedPlayerFlag {

public SavableMapFlagTracker tracker;

public boolean savingNow = false;

public boolean loadingNow = false;
public AtomicBoolean savingNow = new AtomicBoolean(false), loadingNow = new AtomicBoolean(false);

public boolean shouldExpire() {
if (cacheTimeoutSeconds == -1) {
Expand Down Expand Up @@ -103,7 +102,7 @@ public void run() {
}
}
};
if (cache.savingNow || cache.loadingNow) {
if (cache.savingNow.get() || cache.loadingNow.get()) {
new BukkitRunnable() {
@Override
public void run() {
Expand All @@ -121,7 +120,7 @@ public void run() {
}
cache.tracker.modified = false;
String text = cache.tracker.toString();
cache.savingNow = true;
cache.savingNow.set(true);
new BukkitRunnable() {
@Override
public void run() {
Expand All @@ -131,7 +130,7 @@ public void run() {
catch (Throwable ex) {
Debug.echoError(ex);
}
cache.savingNow = false;
cache.savingNow.set(false);
expireTask.runTaskLater(Denizen.getInstance(), 1);
}
}.runTaskAsynchronously(Denizen.getInstance());
Expand All @@ -142,7 +141,7 @@ public static void loadFlags(UUID id, CachedPlayerFlag cache) {
cache.tracker = SavableMapFlagTracker.loadFlagFile(new File(dataFolder, id.toString()).getPath(), false);
}
finally {
cache.loadingNow = false;
cache.loadingNow.set(false);
}
}

Expand All @@ -155,7 +154,7 @@ public static AbstractFlagTracker getTrackerFor(UUID id) {
if (cache != null) {
cache.lastAccessed = CoreUtilities.monotonicMillis();
if (CoreConfiguration.debugVerbose) {
Debug.echoError("Verbose - flag tracker updated for " + id);
Debug.echoError("Verbose - (getTrackerFor) flag tracker updated from soft to main for " + id);
}
playerFlagTrackerCache.put(id, cache);
secondaryPlayerFlagTrackerCache.remove(id);
Expand All @@ -164,9 +163,9 @@ public static AbstractFlagTracker getTrackerFor(UUID id) {
}
cache = new CachedPlayerFlag();
cache.lastAccessed = CoreUtilities.monotonicMillis();
cache.loadingNow = true;
cache.loadingNow.set(true);
if (CoreConfiguration.debugVerbose) {
Debug.echoError("Verbose - flag tracker updated for " + id);
Debug.echoError("Verbose - (getTrackerFor) flag tracker created for " + id);
}
playerFlagTrackerCache.put(id, cache);
loadFlags(id, cache);
Expand All @@ -175,9 +174,15 @@ public static AbstractFlagTracker getTrackerFor(UUID id) {
}
}
else {
if (cache.loadingNow) {
if (CoreConfiguration.debugVerbose) {
Debug.echoError("Verbose - (getTrackerFor) flag tracker was cached for " + id);
}
if (cache.loadingNow.get()) {
long start = CoreUtilities.monotonicMillis();
while (cache.loadingNow) {
while (cache.loadingNow.get()) {
if (CoreConfiguration.debugVerbose) {
Debug.echoError("Verbose - (getTrackerFor) flag tracker is loading, so waiting, for " + id + " ... at " + (CoreUtilities.monotonicMillis() - start) + "ms");
}
if (CoreUtilities.monotonicMillis() - start > 15 * 1000) {
Debug.echoError("Flag loading timeout, errors may follow");
playerFlagTrackerCache.remove(id);
Expand All @@ -200,6 +205,9 @@ public static Future loadAsync(UUID id) { // Note: this method is called sync, b
try {
CachedPlayerFlag cache = playerFlagTrackerCache.get(id);
if (cache != null) {
if (CoreConfiguration.debugVerbose) {
Debug.echoError("Verbose - (loadAsync) flag tracker ignored due to cache for " + id);
}
return null;
}
SoftReference<CachedPlayerFlag> softRef = secondaryPlayerFlagTrackerCache.get(id);
Expand All @@ -208,7 +216,7 @@ public static Future loadAsync(UUID id) { // Note: this method is called sync, b
if (cache != null) {
cache.lastAccessed = CoreUtilities.monotonicMillis();
if (CoreConfiguration.debugVerbose) {
Debug.echoError("Verbose - flag tracker updated for " + id);
Debug.echoError("Verbose - (loadAsync) flag tracker updated from softref to main for " + id);
}
playerFlagTrackerCache.put(id, cache);
secondaryPlayerFlagTrackerCache.remove(id);
Expand All @@ -217,9 +225,9 @@ public static Future loadAsync(UUID id) { // Note: this method is called sync, b
}
CachedPlayerFlag newCache = new CachedPlayerFlag();
newCache.lastAccessed = CoreUtilities.monotonicMillis();
newCache.loadingNow = true;
newCache.loadingNow.set(true);
if (CoreConfiguration.debugVerbose) {
Debug.echoError("Verbose - flag tracker updated for " + id);
Debug.echoError("Verbose - (loadAsync) flag tracker created " + id);
}
playerFlagTrackerCache.put(id, newCache);
CompletableFuture future = new CompletableFuture();
Expand All @@ -228,6 +236,9 @@ public static Future loadAsync(UUID id) { // Note: this method is called sync, b
public void run() {
loadFlags(id, newCache);
Bukkit.getScheduler().scheduleSyncDelayedTask(Denizen.instance, () -> {
if (CoreConfiguration.debugVerbose) {
Debug.echoError("Verbose - flag tracker async loaded " + id);
}
if (newCache.tracker != null) {
newCache.tracker.doTotalClean();
}
Expand All @@ -247,10 +258,10 @@ public static void saveAllNow(boolean canSleep) {
for (Map.Entry<UUID, CachedPlayerFlag> entry : playerFlagTrackerCache.entrySet()) {
CachedPlayerFlag flags = entry.getValue();
if (flags.tracker.modified) {
if (!canSleep && flags.savingNow || flags.loadingNow) {
if (!canSleep && flags.savingNow.get() || flags.loadingNow.get()) {
continue;
}
while (flags.savingNow || flags.loadingNow) {
while (flags.savingNow.get() || flags.loadingNow.get()) {
try {
Thread.sleep(10);
}
Expand Down

0 comments on commit 41ed061

Please sign in to comment.