java.util.ConcurrentModificationException

[Quezler]

Modpack Version

1.1.2

Describe your issue.

pneumaticcraft seems to have a tendency to crash some (but not all) players on a server when someone picks up a drone.

waited till the crash happened to myself before reporting (didn't happen since i placed & picked those drones up, which seems to not crash), but in this case one of the players said he picked up a drone item after unplonking it, the linked log shows me and another player crashing at the same time, but the player that picked the drone up & another player did not crash.

TeamPneumatic/pnc-repressurized#990

Crash Report

https://gist.github.com/Quezler/d228dc4d5ee7bd088e62af8265a06f0a

Latest Log

No response

Have you modified the modpack?

Yes

User Modifications

kubejs atum:artifacts item tag polyfill

Did the issue happen in singleplayer or on a server?

Server

[Quezler]

after trying some of @desht's debug builds the issue was fixed by adding a config option to disable nbt stripping.

this won't fix it by default, but this comment will be found by e6 people searching on github for the exception message.

(going to the linked issue & reading the commit mentions at the bottom should help those people find the config option)

i waited roughly a week before updating this to make sure the crash didn't occur again, and the fix works perfectly. 👍

[NielsPilgaard]

What would we lose, if anything, by switching the config to true by default?

[Quezler]

but potentially increase the amount of network chatter from server to (each) client.

[NielsPilgaard]

Ah ^^' I'm not sure whether that's worth it or not. Do you have any idea as to how frequently this bug appears?

[Quezler]

Well, it appears to happen on multiplayer servers when someone is holding a drone, then other players within render distance have a possibility to crash (to the main menu, its not a hard client crash).

I don't see reports that often (old ones or people just don't bother reporting it or no one uses drones), i'd say inform players about the option so they can toggle it themselves if they encounter it, but good luck on telling absolutely everyone.

but assuming there is only extra data if other players are in range + there is no easy fix in sight it might just be more convenient to have it on by default for everyone, at the cost of some more network traffic, but hey this is modded mc.

some results from the enigmatica discord server, mostly older posts & by members that actually use drones:

edit:

essentially only a problem if players are together when they're playing with drone automation, basing apart of periodically visiting is safe, but i recently also saw the author commit the same fix for armor so perhaps that's borked too 🤔

(not sure if the armor patch is released yet on curseforge)

[NielsPilgaard]

Alrighty, thanks 👍 Based on that, I think it's best to enable it by default.

[desht]

It's liable to affect any item which uses pressure (so drones, armor, minigun...)

From my investigations, it's almost certainly down to some other mod making alterations to item NBT during the IForgeItem#getShareTag() method, which is used to sync NBT to the client (either to add server-only data that the client needs, or strip server data that the client doesn't).

IForgeItem#getShareTag() can run on both the main and network threads of the server, so it's critical that the itemstack NBT is only read, never modified during the method. The correct approach is to copy the item's NBT and modify & return that local copy. If any mod does modify the item's own NBT in that method, any attempt to read it (like I do) is susceptible to a CME, since NBT storage is not thread-safe.

Problem is trying to determine which mod is misbehaving. I've done a quick overview of everything in E6 but I've found nothing obvious as yet. But I'll keep looking and report back if I do spot something.

[Quezler]

The only mod that comes to mind that modifies the nbt of items from other mods is astral sorcery, it has a tendency to add an AS_Amulet_Holder tag to any freshly held weapon or equipped armor, it does not seem to apply outside items in the weapons or armor category, but its the only one i could think of 🤔