Open Parties and Claims

Open Parties and Claims

25M Downloads

Crash involving Frost Walker + OPAC.

Vaelzan opened this issue ยท 11 comments

commented

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?

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())

commented

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.

commented

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.

commented

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.

commented

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.

commented

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

commented

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

commented

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!

commented

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

commented

Made a mistake, the fix hasn't been pushed yet. Going to be very soon.

commented

Forgot to close this one. It's been fixed for a while.