From dc3ed5a77632c954834cc13ac2c4557015c88b75 Mon Sep 17 00:00:00 2001 From: OBro1961 Date: Mon, 16 Dec 2024 11:07:10 -0600 Subject: [PATCH] Better but still bad attempt to fix #181 --- changelog.md | 2 + .../obro1961/chatpatches/chatlog/ChatLog.java | 46 +++++++++++++------ .../chatpatches/mixin/gui/ChatHudMixin.java | 2 +- .../obro1961/chatpatches/util/ChatUtils.java | 2 +- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/changelog.md b/changelog.md index eb7f670..705e218 100644 --- a/changelog.md +++ b/changelog.md @@ -4,6 +4,8 @@ - Now compatible with 1.21.4! Only ONE code change was made and it's super basic :D ([#212](https://www.github.com/mrbuilder1961/ChatPatches/issues/212)) - Updated `pt_br` translations thanks to [demorogabrtz](https://github.com/demorogabrtz)! ([#209](https://www.github.com/mrbuilder1961/ChatPatches/issues/209)) - Fixed players on colored teams not having their names colored when far away ([#202](https://www.github.com/mrbuilder1961/ChatPatches/issues/202)) +- Added a better but still odd preemptive fix for random `ConcurrentModificationException` crashes ([#181](https://www.github.com/mrbuilder1961/ChatPatches/issues/181)) + - Report any bugs relating to this if they occur, as I'm kinda in the dark about it - **Dev notes:** - Removed the now redundant `Flags` class, which helped some random parts of the message modification process that didn't actually need to be out of scope - Removed the casting methods in the two `Accessor` interfaces: we don't need an extra method call just for a cast diff --git a/src/main/java/obro1961/chatpatches/chatlog/ChatLog.java b/src/main/java/obro1961/chatpatches/chatlog/ChatLog.java index a32192c..bf0a41a 100644 --- a/src/main/java/obro1961/chatpatches/chatlog/ChatLog.java +++ b/src/main/java/obro1961/chatpatches/chatlog/ChatLog.java @@ -28,6 +28,7 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.ArrayList; +import java.util.ConcurrentModificationException; import java.util.function.Function; import static obro1961.chatpatches.ChatPatches.LOGGER; @@ -47,7 +48,16 @@ public class ChatLog { public static boolean loaded = false; public static int ticksUntilSave = config.chatlogSaveInterval * 60 * 20; // convert minutes to ticks - private static boolean restoring = false; + /** + * Used to suspend the addition and restoration + * of new messages to the chat log, which + * prevents log spam of restored messages and + * concurrent modification exceptions. + * + * @see #restore() + * @see #serialize() + */ + private static boolean suspended = false; private static ChatLog.Data data = new Data(); private static int lastHistoryCount = -1, lastMessageCount = -1; @@ -217,10 +227,21 @@ public static void serialize() { LOGGER.info("[ChatLog.serialize] Saved the chat log containing {} messages and {} sent messages to '{}' in {} seconds", messageCount(), historyCount(), PATH, (System.currentTimeMillis() - start) / 1000.0 ); + } catch(ConcurrentModificationException cme) { + // this branch is intended to prevent CMEs, with the assumption they are caused by new messages being added while saving + // if the CME is thrown, it will try to save again after the suspension is lifted, otherwise it will dump the data, log + // the error, and unsuspend the chat log. + // warning: not rigorously tested; if this malfunctions and there's no clear solution just delete this, as it's extremely rare + if(suspended) { + LOGGER.error("[ChatLog.serialize] A ConcurrentModificationException occurred while trying to save the chat log:", cme); + dumpData(); + suspended = false; + return; + } - // temporarily removed the ugly ConcurrentModificationException catch block bc it's ugly and not a real solution: - // fixme! - + suspended = true; + serialize(); + suspended = false; } catch(IOException | RuntimeException e) { LOGGER.error("[ChatLog.serialize] An I/O or unexpected runtime error occurred while trying to save the chat log:", e); dumpData(); @@ -249,7 +270,7 @@ public static void backup() { * the {@linkplain ChatHud chat hud}. */ public static void restore() { - restoring = true; + suspended = true; if(!data.history.isEmpty()) data.history.forEach(mc.inGameHud.getChatHud()::addToMessageHistory); @@ -257,7 +278,7 @@ public static void restore() { if(!data.messages.isEmpty()) data.messages.forEach(msg -> mc.inGameHud.getChatHud().addMessage(msg, null, RESTORED_TEXT)); - restoring = false; + suspended = false; LOGGER.info("[ChatLog.restore] Restored {} messages and {} history messages from '{}' into Minecraft!", messageCount(), historyCount(), PATH); } @@ -318,7 +339,7 @@ public static void dumpData() { public static void addMessage(Text msg) { - if(restoring) + if(suspended) return; if(messageCount() > config.chatMaxMessages) data.messages.removeFirst(); @@ -326,7 +347,7 @@ public static void addMessage(Text msg) { data.messages.add(msg); } public static void addHistory(String msg) { - if(restoring) + if(suspended) return; if(historyCount() > config.chatMaxMessages) data.history.removeFirst(); @@ -334,13 +355,8 @@ public static void addHistory(String msg) { data.history.add(msg); } - /** - * Returns if the chat log is currently - * being restored into the chat. Used - * to prevent logging and modifying - * restored messages. - */ - public static boolean isRestoring() { return restoring; } + /** @see #suspended */ + public static boolean isSuspended() { return suspended; } public static void clearMessages() { data.messages.clear(); diff --git a/src/main/java/obro1961/chatpatches/mixin/gui/ChatHudMixin.java b/src/main/java/obro1961/chatpatches/mixin/gui/ChatHudMixin.java index 79295fd..6ef6a26 100644 --- a/src/main/java/obro1961/chatpatches/mixin/gui/ChatHudMixin.java +++ b/src/main/java/obro1961/chatpatches/mixin/gui/ChatHudMixin.java @@ -159,7 +159,7 @@ private boolean disableCommandLog(CommandHistoryManager manager, String message) @Inject(method = "logChatMessage", at = @At("HEAD"), cancellable = true) private void ignoreRestoredMessages(ChatHudLine hudLine, CallbackInfo ci) { - if(ChatLog.isRestoring() && hudLine.indicator() != null) + if(ChatLog.isSuspended() && hudLine.indicator() != null) ci.cancel(); } diff --git a/src/main/java/obro1961/chatpatches/util/ChatUtils.java b/src/main/java/obro1961/chatpatches/util/ChatUtils.java index 326cce3..7c1f374 100644 --- a/src/main/java/obro1961/chatpatches/util/ChatUtils.java +++ b/src/main/java/obro1961/chatpatches/util/ChatUtils.java @@ -165,7 +165,7 @@ public static MutableText buildMessage(@Nullable Style rootStyle, @Nullable Text * */ public static Text modifyMessage(@NotNull Text m, boolean refreshing) { - if( refreshing || ChatLog.isRestoring() ) + if( refreshing || ChatLog.isSuspended() ) return m; // cancels modifications when loading the chatlog or regenerating visibles boolean errorThrown = false;