OreCruncher / DynamicSurroundingsFabric

Dynamic Surroundings mod for Minecraft
MIT License
65 stars 25 forks source link

Client crashes upon clicking "Save and Quit" in game #10

Closed Lorelando closed 9 months ago

Lorelando commented 2 years ago

Upon clicking "Save and Quit" in game, the client crashes. It happens often, but not reliably. It appears to be caused by an interaction with some other mod I have installed, I have however not been able to find out which one it is. The crash only occurs upon quitting the game and also only when Dynamic surroundings is installed.

crash-2021-10-30_15.38.24-client.txt

I'll try to test specifically which interaction is causing the crash. If you have an idea, what might be causing it, I'm quite willing to test it.

JustCat80 commented 2 years ago

Can reproduce, but not sure of the exact cause as it seems slightly random, i was attempting to narrow it down to a specific mod but when i added them all back it works fine. Then after reopening, moving a bit, and attempting to save again, it crashes

JustCat80 commented 2 years ago

It seems to be whenever the music kicks in but i am not 100% sure

JustCat80 commented 2 years ago

Yep it is, was able to reproduce with only Mod menu, fabric api, and Dynamic surroundings I was underground and waited for the music to start playing, once it starts playing and you attempt to save and quit, the game crashes. crash-2021-12-13_13.32.03-client.txt So, its not a mod, its just the music

ThexXTURBOXx commented 2 years ago

I was able to reproduce this with Not Enough Crashes installed, which gives a better stack trace: https://pastebin.com/xLJA62ci

Also, I have music disabled. Thus, it can't be the music in my case here and it needs to be something else

HarvelsX commented 2 years ago

Here's an "understandable" log for you. I hope it helps, but it didn't help me much. It happens because the Source being closed in SourceManager from Channel does not exist in SoundEngine, which can only be created through Channel, which exactly registers it in SoundEngine, which causes the crash. Hmmm, I'm looking here and it seems it can only be when Source in SourceManager is null, needs research on this problem!

Also try testing this with the standard Client.Config.soundSystem.streamingChannels parameter (streamingChannels) in 8.

I'm sure you will solve this problem quickly!

debug-1.log

HarvelsX commented 2 years ago

It can't be null either, maybe we should consider org/orecruncher/dsurround/mixins/audio/MixinSource.java?

ThexXTURBOXx commented 2 years ago

It can't be null either, maybe we should consider org/orecruncher/dsurround/mixins/audio/MixinSource.java?

I already looked at that one. I am pretty sure there is a unregistered channel (or one that is not correctly registered)

HarvelsX commented 2 years ago

I can not figure out what is causing this, I hope you can solve this problem. I will be very grateful to you for an explanation of the solution to this problem!

HarvelsX commented 2 years ago

I already looked at that one. I am pretty sure there is a unregistered channel (or one that is not correctly registered) Why do you think so?

It's just that Minecraft itself creates one Channel instance, and the error says that Channel.sources has elements that aren't in SoundEngine.(streaming/static)Source, which shouldn't be and can't seem to be.

The Source itself can be null, by the @Nullable annotation inside SourceManager. I can assume that the Source at the moment of SoundEngine.release is null.

Maybe I don't understand something?

ThexXTURBOXx commented 2 years ago

I can assume that the Source at the moment of SoundEngine.release is null.

Nope, I already looked at that. Using a Mixing at HEAD of release(), which prints out the source to release, there is no null source

HarvelsX commented 2 years ago

Oh, okay

ThexXTURBOXx commented 2 years ago

I think, I am now pretty sure I know what is going on: The release function is called somewhere on the render thread, which should not be done. That leads to the channels being released two times since the threads are not synchronized (they don't need to as this shouldn't be done on the render thread anyways). So, sometimes the instances are being released twice because they still remain in the SourceSet despite being already released due to the bad synchronization. I am now trying to track down the faulty call to release.

Here you can see what I mean (RH is at the HEAD of the release function, RT at the TAIL): grafik

HarvelsX commented 2 years ago

Heh, I see, thank you very much for the explanation!

ThexXTURBOXx commented 2 years ago

Opened #17 with a workaround (not a fix!)

HarvelsX commented 2 years ago

Is it possible that after this.taskQueueue.restart(); the modification tries to play the audio while ignoring the playback stop? It looks like the audio processing in this.taskQueue is happening while this.channel.close();, and at that time the processing in this.taskQueue in the Local Client thread is simultaneously with this.channel.close(); in the Render thread trying release.

this.channel.close(); in a Render thread for the Minecraft is more than normal practice