Skip to content

[1.21.1] 6.0.10: ConcurrentModificationException in AllSoundEvents.prepare() #10735

Description

@SpaceOddityTr

Environment: Create 6.0.10, Minecraft 1.21.1, NeoForge 21.1.248, in a pack where seventeen mods reference AllSoundEvents (Create Big Cannons and CBC Military Supplement among them). Seen on the client; Create.onCtor runs the same code on a dedicated server.

What happens

Intermittently, the game fails to launch: AllSoundEvents.prepare() throws ConcurrentModificationException inside Create's own mod constructor, and FML aborts create:

[modloading-worker-0/ERROR] [net.neoforged.fml.javafmlmod.FMLModContainer/LOADING]:
    Failed to create mod instance. ModID: create, class com.simibubi.create.Create
java.util.ConcurrentModificationException: null
    at java.base/java.util.HashMap$HashIterator.nextNode(HashMap.java:1605)
    at java.base/java.util.HashMap$ValueIterator.next(HashMap.java:1633)
    at create@6.0.10/com.simibubi.create.AllSoundEvents.prepare(AllSoundEvents.java:387)
    at create@6.0.10/com.simibubi.create.Create.onCtor(Create.java:115)
    at create@6.0.10/com.simibubi.create.Create.<init>(Create.java:106)

Because this happens in the mod constructor, FML aborts before registry initialisation. Every DeferredHolder in the game then stays unbound, and any mod that dereferences one of its own holders while the error screen is being built turns NeoForge's recoverable "mod loading errors" screen into a hard crash that names an unrelated mod. In my case the reported crash was an NPE inside a music mod, which sent me looking in entirely the wrong place — worth knowing, because it makes this defect very hard to attribute to Create.

Expected

prepare() completes regardless of an add-on publishing a sound entry while the loop is running.

Steps to reproduce

I have no deterministic reproducer. It showed up on a pack with several Create add-ons installed, twice within two days, with an identical stack both times — roughly one launch in two on that pack. It does not reproduce with Create alone.

Evidence

AllSoundEvents.ALL is a plain, public, mutable HashMap:

public static final Map<ResourceLocation, SoundEntry> ALL = new HashMap<>();

and SoundEntryBuilder.build() ends by publishing into it:

SoundEntry entry = /* CustomSoundEntry or WrappedSoundEntry */;
ALL.put(entry.getId(), entry);
return entry;

So every add-on that builds a Create sound entry writes into Create's own global map, whether or not it also keeps its own copy. That is the documented way to add Create sounds, and add-ons use it: in my pack seventeen mods reference AllSoundEvents, and at least three ship their own SoundEntryBuilder subclass with its own map (CNASounds, CBCMSSoundEvents, CreateBigCannonsNeoForge) — all of which still land in ALL, because the subclasses inherit build().

prepare() then iterates that same map:

public static void prepare() {
    for (SoundEntry entry : ALL.values())
        entry.prepare();
}

Any put into ALL between the first values().iterator() call and the end of the loop makes HashMap throw. The window is small, which is why it only reproduces sometimes, and it widens with every Create add-on installed.

Suggested fix

Any one of these closes it:

  1. Iterate a copy: for (SoundEntry entry : new ArrayList<>(ALL.values())) entry.prepare();
    The one-line version. It stops the crash but silently skips an entry added during the loop.
  2. Iterate a copy and then drain: repeat with whatever is in ALL but not yet prepared, until nothing new appears. Note that WrappedSoundEntry.prepare() appends to compiledEvents without clearing it, so an entry must be prepared exactly once — a naive re-run over the whole map would duplicate every compiled event.
  3. Make ALL a ConcurrentHashMap. This also covers add-ons that build sound entries from a thread other than the loading thread, which the copy alone does not.

Making ALL unmodifiable from outside and giving add-ons an explicit registration call would be the cleaner long-term shape, but it would break existing add-ons.

For what it is worth, I work around it in my pack from the outside with a small mixin: it redirects the ALL.values() call inside prepare() to a defensive copy, and after Create's loop prepares any entry that appeared meanwhile, tracking entries by identity so none is prepared twice. That is option 2 above, applied externally.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type: crashIssue where the game crashes

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions