Crash involving Frost Walker + OPAC.

[Vaelzan]

I've just been sent this crash report by one of our players. I hadn't seen it before today so I'm not sure how they ended up in this state. Unfortunately I can't get them to try with the latest version of OPAC right now, but looking at the latest source code here in the repo it seems like there might still be a fix that can be made.

https://bytebin.lucko.me/gIvsGSpsNX

I have no idea how FROSTWALK_ENTITY ended up nonnull but FROSTWALK_LEVEL is null. That seems weird to me since they appear to be both set at the same time on consecutive lines. It looks like the crash is happening during one of the Ensorcellation mod's events, but I haven't had a chance to look into why that's happening.

Is it worth adding a null safety check on FROSTWALK_LEVEL here?

open-parties-and-claims/Common/src/main/java/xaero/pac/common/server/core/ServerCore.java

Line 573 in 460ae7c

if(FROSTWALK_ENTITY == null || FROSTWALK_LEVEL.getServer() == null || !FROSTWALK_LEVEL.getServer().isSameThread())

ie. changing line 573 from if(FROSTWALK_ENTITY == null || FROSTWALK_LEVEL.getServer() == null || !FROSTWALK_LEVEL.getServer().isSameThread())

to if(FROSTWALK_ENTITY == null || FROSTWALK_LEVEL == null || FROSTWALK_LEVEL.getServer() == null || !FROSTWALK_LEVEL.getServer().isSameThread())

[thexaero]

It's not possible for FROSTWALK_ENTITY to be nonnull while FROSTWALK_LEVEL is null. A null level would cause a crash in preFrostWalkHandle before ever setting the entity.
I'm not sure if it was in the older version that they are using.

[thexaero]

Looks like it wasn't possible back then too. This is a strange one. I don't think a null check would even fully help because it's basically null-checked as is.

[Vaelzan]

It's not possible for FROSTWALK_ENTITY to be nonnull while FROSTWALK_LEVEL is null. A null level would cause a crash in preFrostWalkHandle before ever setting the entity. I'm not sure if it was in the older version that they are using.

I mean that's literally what the stacktrace says is going on isn't it? I don't think I'm reading it wrong. Looking at the same section of code from that period of time via the commit history it doesn't look like it has changed since then either.

[thexaero]

Alright, I think I see what's happening. The crash is happening on the client side. It's a race condition with the server thread. I'll have to make sure preBlockStateFetchOnFrostwalk only runs on the server thread.

[thexaero]

Not sure if it's a compatibility issue or fully on my end, but I'm leaning towards a compatibility issue with ensorcellation.

[Vaelzan]

Link to the relevant Ensorcellation code, if it helps: https://github.com/CoFH/Ensorcellation/blob/f5ebfc1c8913d84c59e7ac7e91fbd145d792768f/src/main/java/cofh/ensorcellation/event/CommonEvents.java#L346

[thexaero]

Thanks! Yeah, probably related. Either way though, I was definitely intending to catch non-server-thread calls and didn't do it right.

[Vaelzan]

Heh, of course when things that are meant to be impossible happen it's because of multithreading. Almost every time. :)

I had a feeling it might have been something like that; the player who reported it to me mentioned that it wasn't consistently crashing, just occasionally (as well as no other players ever mentioning such a crash). For now they're happy enough just using a different enchant to avoid it.

Thanks for taking a look!

[thexaero]

The issue should only affect integrated servers (mostly singleplayer) and occur very rarely. I'll try to find time to fix it ASAP though.