NullPointerException at `ModelLoadingEventDispatcher.modifyModelBeforeBake`

[Tictim]

Playing FTB Evolution. Crash log: https://pastebin.com/Vid89qXr

Seemingly the crash happened on RS's cable block, but I don't think it is the culprit as code ultimately flows into aforementioned method and has little to do with this specific crash at that point.

What puzzles me is that the the crash seems impossible to occur. Probably some race condition stuff I do not know about (can ObjectArrayList just return null???) or reflection voodoo gone wrong, if I have to guess, but I figured I'd report it anyway for the sake of documentation.

[Su5eD]

This is pretty strange indeed. I'll close the issue for now as I can't reproduce the crash, but if it comes up again let me know.

[jerezereh]

Following up on this with a reliable repro and root cause. It's a data race on the dispatcher's shared context stacks — which is why it looked "impossible" and wouldn't reproduce on demand. I've now hit it three times with three different exceptions, all the same underlying bug, including the exact NPE in this issue's title.

Root cause: the modifier context stacks aren't thread-safe

ModelLoadingEventDispatcher keeps its context stacks as plain shared instance fields (1.21.1 branch):

private final ObjectArrayList<ModelResolverContext>      modelResolverContextStack      = new ObjectArrayList<>();
private final ObjectArrayList<OnLoadModifierContext>     onLoadModifierContextStack     = new ObjectArrayList<>();
private final ObjectArrayList<BeforeBakeModifierContext> beforeBakeModifierContextStack = new ObjectArrayList<>();
private final ObjectArrayList<AfterBakeModifierContext>  afterBakeModifierContextStack  = new ObjectArrayList<>();

modifyModelBeforeBake / modifyModelAfterBake do a check-then-act on these — isEmpty(), then pop(), use, then push(). That's safe only if baking happens on one thread (the resource-reload flow). But some mods bake lazily at render time on worker threads: Refined Storage's CableBakedModel.getQuads, on a Guava LoadingCache miss, calls back into ModelBakery$ModelBakerImpl.bake(...) -> the wrapInnerBake mixin. When two of those run concurrently, the shared stacks corrupt.

I've encountered three crashes, all on modifyModelBeforeBake / modifyModelAfterBake, all from CableBakedModel baking on a Sodium chunk-mesh worker:

Exception	Where	Interleaving that produces it
IndexOutOfBoundsException: Index (0) >= size (1)	modifyModelBeforeBake pop	two threads pop, size desyncs
ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 10	modifyModelAfterBake pop	concurrent pop drives size below 0
NullPointerException: … context is null	modifyModelBeforeBake (context.prepare(...))	isEmpty() passes, then another thread pops first, so this thread's pop() reads a slot the other just nulled

That last one is the NPE from this issue's title.

Reliable repro: Sodium + Refined Storage on a multi-core CPU

1. NeoForge 1.21.1, Forgified Fabric API + Sodium + Refined Storage 2.x.
2. Multi-core CPU; Sodium chunk_builder_threads = 0 (auto -> many workers). Higher core count = fires more readily.
3. Build a decent-sized RS cable network of multiple cable variants, then repeatedly force those chunks to re-mesh (teleport in/out of range, relog) with a cold model cache.

It's a per-unit-time hazard: it won't fire every attempt, but it will within a handful. No other mod is required; a second concurrent baker such as Distant Horizons' LOD builder raises the rate, but I reproduced it with DH's distant generation off, so Sodium's own worker concurrency alone is sufficient. Guava serializes concurrent misses on the same key, so it's concurrent bakes of distinct cable models across workers that collide.

Confirmed mitigation: Sodium chunk_builder_threads = 1 eliminates the concurrency at a generally small performance hit.

RS considers the render-time lazy bake intentional and won't change it (refinedmods/refinedstorage#3763, refinedmods/refinedstorage2#1255), so I believe the fix needs to be on this side.

Suggested fix

Make the four context stacks thread-local (ThreadLocal<ObjectArrayList<…>>), or otherwise scope them per baking thread. The push in wrapInnerBake and the pop/use/push in the modify hooks are already symmetric within a single thread, so per-thread stacks should keep each thread self-consistent without cross-thread interference.

Happy to test a snapshot build against the repro above.