Mod unsafely accesses client world off-thread #172

embeddedt · 2023-11-14T02:51:18Z

Bug description

Sound Physics Remastered accesses the client world from the sound thread. The client world is not really thread-safe and doing this can lead to all sorts of issues. In my case, it seems to cause ghost Create mechanical belts to occasionally render in the world with Embeddium. I am not 100% sure of the exact action that causes that issue, but it seems related to block retrieval off-thread triggering creation of block entities as well as other side effects. A stack trace from my debug mixin is attached below so you can see what I mean (it's using SRG names, sorry about that).

To fix this issue properly I believe the mod will need to be refactored so that all block retrieval is done on the main thread and stored in a snapshot data structure that the sound thread then accesses. Otherwise, this will likely continue to cause all sorts of subtle problems that will be very difficult for players/other mod authors to track down.

This issue was discovered on 1.20.1 but likely affects earlier versions too unless the code changed significantly.

java.lang.Exception: null
	at net.minecraft.world.level.chunk.LevelChunk.handler$zbb000$yellForOffThread(LevelChunk.java:1338) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading,pl:mixin:APP:flywheel.mixins.json:instancemanage.InstanceAddMixin,pl:mixin:APP:rubidium.mixins.json:core.render.world.WorldMixin,pl:mixin:A}
	at net.minecraft.world.level.chunk.LevelChunk.m_62934_(LevelChunk.java) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading,pl:mixin:APP:flywheel.mixins.json:instancemanage.InstanceAddMixin,pl:mixin:APP:rubidium.mixins.json:core.render.world.WorldMixin,pl:mixin:A}
	at net.minecraft.world.level.chunk.LevelChunk.m_5685_(LevelChunk.java:315) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading,pl:mixin:APP:flywheel.mixins.json:instancemanage.InstanceAddMixin,pl:mixin:APP:rubidium.mixins.json:core.render.world.WorldMixin,pl:mixin:A}
	at net.minecraft.world.level.Level.m_7702_(Level.java:560) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B}
	at com.simibubi.create.foundation.block.IBE.getBlockEntity(IBE.java:72) ~[create-1.20.1-0.5.1.f.jar%23151!/:0.5.1.f] {re:classloading}
	at com.simibubi.create.foundation.block.IBE.getBlockEntityOptional(IBE.java:53) ~[create-1.20.1-0.5.1.f.jar%23151!/:0.5.1.f] {re:classloading}
	at com.simibubi.create.content.kinetics.belt.BeltBlock.m_5939_(BeltBlock.java:370) ~[create-1.20.1-0.5.1.f.jar%23151!/:0.5.1.f] {re:classloading}
	at net.minecraft.world.level.block.state.BlockBehaviour$BlockStateBase.m_60742_(BlockBehaviour.java:684) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B,pl:mixin:APP:modernfix-common.mixins.json:bugfix.chunk_deadlock.BlockStateBaseMixin,pl:mixin:APP:ferritecore.blockstatecache.mixin.json:BlockStateBaseMixin,pl:mixin:APP:modernfix-common.mixins.json:perf.reduce_blockstate_cache_rebuilds.BlockStateBaseMixin,pl:mixin:A}
	at net.minecraft.world.level.ClipContext$Block.m_7544_(ClipContext.java:62) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:classloading}
	at net.minecraft.world.level.ClipContext.m_45694_(ClipContext.java:40) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:classloading}
	at net.minecraft.world.level.BlockGetter.m_151358_(BlockGetter.java:63) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading}
	at net.minecraft.world.level.BlockGetter.m_151361_(BlockGetter.java:119) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading}
	at net.minecraft.world.level.BlockGetter.m_45547_(BlockGetter.java:58) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading}
	at com.sonicether.soundphysics.SoundPhysics.raycast(SoundPhysics.java:618) ~[soundphysics-forge-1.20.1-1.2.1.jar%23155!/:1.20.1-1.2.1] {re:mixin,re:classloading}
	at com.sonicether.soundphysics.SoundPhysics.evaluateEnvironment(SoundPhysics.java:287) ~[soundphysics-forge-1.20.1-1.2.1.jar%23155!/:1.20.1-1.2.1] {re:mixin,re:classloading}
	at com.sonicether.soundphysics.SoundPhysics.processSound(SoundPhysics.java:180) ~[soundphysics-forge-1.20.1-1.2.1.jar%23155!/:1.20.1-1.2.1] {re:mixin,re:classloading}
	at com.sonicether.soundphysics.SoundPhysics.onPlaySound(SoundPhysics.java:147) ~[soundphysics-forge-1.20.1-1.2.1.jar%23155!/:1.20.1-1.2.1] {re:mixin,re:classloading}
	at com.mojang.blaze3d.audio.Channel.handler$zjj000$play(Channel.java:533) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading,pl:mixin:APP:assets/sound_physics_remastered/sound_physics_remastered.mixins.json:SourceMixin,pl:mixin:APP:assets/sound_physics_remastered/sound_physics_remastered.mixins.json:ChannelAccessor,pl:mixin:A}
	at com.mojang.blaze3d.audio.Channel.m_83672_(Channel.java) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading,pl:mixin:APP:assets/sound_physics_remastered/sound_physics_remastered.mixins.json:SourceMixin,pl:mixin:APP:assets/sound_physics_remastered/sound_physics_remastered.mixins.json:ChannelAccessor,pl:mixin:A}
	at net.minecraft.client.sounds.SoundEngine.lambda$play$6(SoundEngine.java:404) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,pl:accesstransformer:B,pl:runtimedistcleaner:A,re:classloading,pl:accesstransformer:B,pl:mixin:APP:assets/sound_physics_remastered/sound_physics_remastered.mixins.json:SoundSystemMixin,pl:mixin:A,pl:runtimedistcleaner:A}
	at net.minecraft.client.sounds.ChannelAccess$ChannelHandle.m_120157_(ChannelAccess.java:34) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,re:classloading}
	at net.minecraft.util.thread.BlockableEventLoop.m_6367_(BlockableEventLoop.java:156) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,pl:accesstransformer:B,re:computing_frames,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B}
	at net.minecraft.util.thread.BlockableEventLoop.m_7245_(BlockableEventLoop.java:130) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,pl:accesstransformer:B,re:computing_frames,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B}
	at net.minecraft.util.thread.BlockableEventLoop.m_18701_(BlockableEventLoop.java:139) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:mixin,pl:accesstransformer:B,re:computing_frames,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B}
	at net.minecraft.client.sounds.SoundEngineExecutor.m_120336_(SoundEngineExecutor.java:42) ~[client-1.20.1-20230612.114412-srg.jar%23156!/:?] {re:classloading}
	at java.lang.Thread.run(Thread.java:840) ~[?:?] {re:mixin}

Mods installed

NeoForge 47.1.79
Embeddium 0.2.9
Create 0.5.1f
FerriteCore 6.0.1
ModernFix 5.9.3
Sound Physics Remastered 1.20.1-1.2.1

The text was updated successfully, but these errors were encountered:

henkelmax · 2023-12-14T15:50:19Z

Please provide the full log files of this crash.
Please also make sure this also happens in the latest version of the mod.

embeddedt · 2023-12-14T15:55:13Z

This isn't a specific crash report. It's a technical explanation of a design flaw that I believe is responsible for causing random glitches or crashes in other mods, plus an example stacktrace outlining how the problem occurs.

CelestialAbyss · 2023-12-14T20:41:38Z

Here's a screenshot of one of the issues encountered on 1.19.2 after installing SPR:

henkelmax · 2023-12-16T13:45:49Z

The mod did always access the client world from another thread. This was already a thing back then for the original mod from Sonic Ether. The mod is designed around being able to do this. I also think this mod wouldn't be feasible if blocks need to be accessed from the main thread. If all evaluation rays need to be casted in the main thread, the game frame rate would be unplayable. For now this has always worked without any issues. Its definitely possible that this causes the issues with the create mod, but usually issues from non-thread safe operations are way more inconsistent, so I'm assuming create does some very weird hacky stuff to the world rendering or changes very fundamental stuff. I also always used the mod in conjunction with Sodium and never had any issues. Maybe Embeddium does some things differently.
Interactions from off thread are also limited to read-only calls, which in general shouldn't cause many issues.

All I can do is declare Create or Embeddium as an incompatible mod, as its impossible to make the mod "Thread safe". If anyone has any ideas on how to work around this issue, please let me know, but Thread safety isn't really possible to my knowledge.

embeddedt · 2023-12-16T16:21:51Z

If anyone has any ideas on how to work around this issue, please let me know, but Thread safety isn't really possible to my knowledge.

The only way to do it correctly (as far as I know) is to make a snapshot of the needed block data on the main thread, and then work with that on the sound thread (without ever accessing the client world directly). This is the same approach vanilla uses to update the chunk meshes using worker threads (16x16x16 snapshots are made on main and then used by the workers).

Anyways, the next release of Embeddium will warn users that support won't be provided if Sound Physics (or other unsafe threading mods like Entity Culling) are installed, as I'm getting too many concurrency-related bug reports which are not caused by me, and that I don't have an easy way to fix.

IThundxr · 2024-01-09T21:29:28Z

This also causes https://mclo.gs/cWFrIAu by creating a CME when light updater is invoked since it is not thread safe

MehVahdJukaar · 2024-03-17T16:07:43Z

Why not just wrapping the level to avoid unsafe calls such as getBlockEntity? That seems to be what's causing the issue above

IThundxr · 2024-03-24T15:42:53Z

Marked SPR as broken in create fabric due to the mass amounts of concurrency issues we've gotten, If the suggestion above works and fixes this please let me know so i can remove the breaks.

NolanHewitt · 2024-03-30T07:58:51Z

The suggestion above looks like it would fix it to me. But only way to know for sure is for it to be implemented and tested.

PaulRV0709 · 2024-04-08T11:04:47Z

The create latest patch actually added an incompatibility flag for this mod, so now you can't play with both installed because of this non thread safety.

henkelmax · 2024-04-08T11:25:53Z

The create latest patch actually added an incompatibility flag for this mod, so now you can't play with both installed because of this non thread safety.

Yes, as mentioned here:

#172 (comment)

augustsaintfreytag · 2024-04-09T22:49:34Z

Sound Physics is an amazing mod and is definitely worth it to see this change implemented. I'd say it gives it some solid future-proofing to use an access-by-snapshot approach, especially if vanilla already does it this way.

PeglinX · 2024-04-20T18:32:08Z

just to be clear. Is this planned to be worked on?

Spiderach · 2024-04-21T21:21:06Z

just to be clear. Is this planned to be worked on?

Looks like it was reopened, pinned, and marked as a bug. If not immediately it looks like there will at least be an attempt to fix it.

henkelmax · 2024-04-22T06:27:16Z

Looks like it was reopened, pinned, and marked as a bug. If not immediately it looks like there will at least be an attempt to fix it.

Yes, I currently don't have the capacity to look into it, but its on my list. If anyone is interested to take a look at it or make a PR, any help is always appreciated!

henkelmax · 2024-05-26T14:15:27Z

This should already be fixed with 1.4.2

henkelmax · 2024-05-26T14:20:45Z

In case it still happens, I'd appreciate if you'd send me logs of that version again.

Fyoncle · 2024-06-03T23:35:47Z

What about Create compatibility? Will it be fixed or no

IThundxr · 2024-06-03T23:37:32Z

What about Create compatibility? Will it be fixed or no

that's fixed in dev already

Fyoncle · 2024-06-03T23:38:30Z

What about Create compatibility? Will it be fixed or no

that's fixed in dev already

Oh yea just noticed, sorry. Seems like we need to create a config to fix it, thats probably now in Create's side since they marked the mod as broken.

henkelmax · 2024-07-12T07:21:20Z

This is out for a while now and it seems that there are no major issues, so I'll close this. Thanks again to everyone that helped with this issue!

AClon314 · 2024-07-28T11:06:46Z

@Fyoncle

For anyone who want to do a quick fix: Fabricators-of-Create/Create#1503 (comment)

tajemniktv · 2024-08-22T12:46:53Z

This should already be fixed with 1.4.2

@henkelmax
Hey, oddly the issue might be back? Not sure, but the logs do seem similiar. The game freezes on loading into the server, but after (some, sometimes about 3minutes, it loads fine) https://mclo.gs/XXNEB5w

henkelmax · 2024-08-22T17:39:14Z

[14:44:05] [Sound engine/WARN]: [LevelAccessUtils#getClientLevelProxy] Can not return client level proxy, client level clone has not been cached. This might only occur once on load.

@augustsaintfreytag there might be an issue with the caching in combination with a certain mod.

@tajemniktv Could you check which mod in combination with SPR causes the issue?

Kirrasu · 2024-08-25T23:48:43Z

So I've got the newest version of Create Fabric and Sound Physics Remastered and I'm getting a error that doesn't let the game start at all. I've tried troubleshooting and looking through this thread and one that was marked as spam and directed to this one, but I haven't found anything. I dunno what to do or if there is a version of either mod that will allow me to use the other.

embeddedt · 2024-08-26T00:05:07Z

Create may not have made a release removing the incompatible declaration yet.

IThundxr · 2024-08-26T14:06:10Z

Correct, use a dependency override like people have mentioned above or wait for the next patch

jacklollz2 · 2024-09-09T14:31:46Z

How would I make this dependency override on quilt?

Yummy-cookie · 2024-09-16T01:45:14Z

forge create0.5.1h also has this bug

Yummy-cookie · 2024-09-16T01:49:15Z

forge create0.5.1h also has this bug

1.19.2-latest

PeglinX · 2024-09-16T01:51:56Z

forge create0.5.1h also has this bug

1.19.2-latest

It is known, create only has to update now for it to work. There’s a temporal fix as stated above by making an override file

Yummy-cookie · 2024-09-16T01:53:11Z

forge create0.5.1h也有这个bug

1.19.2-最新

众所周知，create 现在只需更新即可工作。如上所述，可以通过创建覆盖文件来临时修复

how？

PeglinX · 2024-09-16T01:56:42Z

I’m not sure. Check the messages above and see if one of the possible solutions apply to your case

Yummy-cookie · 2024-09-16T01:58:39Z

我不确定。检查上面的消息，看看是否有一个可能的解决方案适用于你的情况

I looked and didn't find a way to forge

henkelmax · 2024-09-16T06:51:31Z

I believe Forge doesn't have a similar thing to dependency overrides.

augustsaintfreytag · 2024-09-16T10:08:26Z

Only just noticed the discussion here, is there a confirmed deadlock issue in SPR? I'd say it could only happen in the caching logic, the only part that happens on the main thread. And it's not like building an initial cache would take longer than any future pass. I personally haven't encountered anything suspicious yet and I do play with quite a number of mods. If people are having issues (@KostromDan), I could take a look, though, if this happens in SPR > 1.4.0.

@tajemniktv If this is actually a server log, none of the sound physics code should actually be running server-side, so that'd be the heart of the issue. That line is logged if a sound is played before the main thread mixin had a chance to cache a radius of the client world which it should always do the first tick. If this runs server-side, there never would be a cache to access so I can picture this just being logged eternally, for every played sound. Not sure why it would cause a lock-up, though. I see this is 1.21.x, there may have been changes I don't know about, haven't touched that version.

On the Create topic, at least on 1.20.1 releases for Fabric, the compatibility rule has been updated for months already, we've had maintainers of Create in this thread following along. I can only imagine this being a problem for older versions or forks of Create.

embeddedt added the triage label Nov 14, 2023

embeddedt assigned henkelmax Nov 14, 2023

embeddedt mentioned this issue Nov 14, 2023

[1.20.1] Create 0.5.1e contraptions visual glitches FiniteReality/embeddium#97

Closed

henkelmax closed this as not planned Won't fix, can't repro, duplicate, stale Dec 16, 2023

henkelmax added bug Something isn't working wontfix This will not be worked on and removed triage labels Dec 16, 2023

TropheusJ mentioned this issue Feb 8, 2024

Threading issues with Sound Physics Remastered crash flywheel Fabricators-of-Create/Create#1244

Closed

henkelmax reopened this Apr 8, 2024

henkelmax pinned this issue Apr 8, 2024

henkelmax mentioned this issue Apr 8, 2024

Incompatible with latest create verision #200

Closed

henkelmax removed the wontfix This will not be worked on label Apr 8, 2024

IThundxr mentioned this issue Apr 8, 2024

Incompatible with Sound Physics Remastered after v0.5.1f Patch 1 update Fabricators-of-Create/Create#1432

Closed

henkelmax mentioned this issue Apr 9, 2024

Fabric incompatible with new Create 0.5.1f #201

Closed

IThundxr mentioned this issue Apr 17, 2024

Sounds Physics Remastered Fabricators-of-Create/Create#1450

Closed

Fyoncle mentioned this issue Jun 3, 2024

Revert SPR Marked As Broken Fabricators-of-Create/Create#1512

Closed

henkelmax mentioned this issue Jun 21, 2024

Create mod dont work with sound physics remastered, #220

Closed

henkelmax closed this as completed Jul 12, 2024

This was referenced Jul 22, 2024

Incompatible with Create Fabric #238

Closed

Long player sounds are cut short on death #233

Closed

henkelmax mentioned this issue Aug 20, 2024

Incompatible with create #245

Closed

tajemniktv mentioned this issue Aug 22, 2024

Sounds Mod Lag With SPR? IMB11/Sounds#103

Closed

1 task

henkelmax mentioned this issue Sep 3, 2024

Sound Physics Not Compatible #247

Closed

IThundxr mentioned this issue Oct 20, 2024

Incompatibility with Starlight Fabricators-of-Create/Create#1629

Closed

Mod unsafely accesses client world off-thread #172

Mod unsafely accesses client world off-thread #172

Comments

embeddedt commented Nov 14, 2023

henkelmax commented Dec 14, 2023

embeddedt commented Dec 14, 2023

CelestialAbyss commented Dec 14, 2023 • edited Loading

henkelmax commented Dec 16, 2023

embeddedt commented Dec 16, 2023

IThundxr commented Jan 9, 2024

MehVahdJukaar commented Mar 17, 2024

IThundxr commented Mar 24, 2024 • edited Loading

NolanHewitt commented Mar 30, 2024

PaulRV0709 commented Apr 8, 2024

henkelmax commented Apr 8, 2024 • edited Loading

augustsaintfreytag commented Apr 9, 2024

PeglinX commented Apr 20, 2024

Spiderach commented Apr 21, 2024

henkelmax commented Apr 22, 2024

henkelmax commented May 26, 2024

henkelmax commented May 26, 2024

Fyoncle commented Jun 3, 2024

IThundxr commented Jun 3, 2024

Fyoncle commented Jun 3, 2024 • edited Loading

henkelmax commented Jul 12, 2024

AClon314 commented Jul 28, 2024

tajemniktv commented Aug 22, 2024 • edited Loading

henkelmax commented Aug 22, 2024

Kirrasu commented Aug 25, 2024

embeddedt commented Aug 26, 2024

IThundxr commented Aug 26, 2024

jacklollz2 commented Sep 9, 2024

Yummy-cookie commented Sep 16, 2024

Yummy-cookie commented Sep 16, 2024

PeglinX commented Sep 16, 2024

Yummy-cookie commented Sep 16, 2024

PeglinX commented Sep 16, 2024

Yummy-cookie commented Sep 16, 2024

henkelmax commented Sep 16, 2024

augustsaintfreytag commented Sep 16, 2024 • edited Loading

CelestialAbyss commented Dec 14, 2023 •

edited

Loading

IThundxr commented Mar 24, 2024 •

edited

Loading

henkelmax commented Apr 8, 2024 •

edited

Loading

Fyoncle commented Jun 3, 2024 •

edited

Loading

tajemniktv commented Aug 22, 2024 •

edited

Loading

augustsaintfreytag commented Sep 16, 2024 •

edited

Loading