diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.09k stars 794 forks source link

[3DS] Game crashes every time a save occurs #1329

Closed warnwar closed 3 years ago

warnwar commented 3 years ago

Important information On New 3DS with sys ver. 11.10.0-43U, using build 3965 from CircleCI https://app.circleci.com/pipelines/github/diasurgical/devilutionX/3965/workflows/62a48a74-b2ee-4e5a-84e0-d4c5052ce6f2/jobs/24941

Describe Any time the game saves, it crashes. The save does complete, but the 3DS turns off because of the crash.

It happens in the menu when creating a character, and it also happens whenever you save from within the game. I have included the crash dumps. crashdump.zip

Expected behavior Game should not crash when saving

AJenbo commented 3 years ago

@warnwar what was the last build that didn't crash?

It happens in the menu when creating a character, and it also happens whenever you save from within the game. I have included the crash dumps. crashdump.zip

Thia looks like an empty folder?

@MrHuu are you able to see what is going on here?

warnwar commented 3 years ago

oops, sorry about uploading the wrong zip for the crashdumps. This one should actually have them: crashdump.zip

This is my first time trying devilution on 3ds, so I can try going back a handful of versions if that will help.

AJenbo commented 3 years ago

Could you try this version https://24485-143324737-gh.circle-artifacts.com/0/devilutionx.cia

what model do you have, i belive some of the early ones are not able to fully run the game.

MrHuu commented 3 years ago

Tested both latest .3dsx and .cia circle-ci builds on my new3ds. Both builds crash after saving.

I'll update my local builds and look into it.

Edit: arm-none-eabi-gdb backtrace:

#0  0x0031ab5c in _free_r ()
#1  0x001492ac in dvl::SMemFree (logfilename=0x3876d0 "Source/engine.cpp", logline=691, defaultValue=0 '\000', 
    location=0x867cd28) at /home/mrhuu/SharedFolder/_git/devilutionX/SourceX/storm/storm.cpp:260
#2  dvl::mem_free_dbg (p=0x867cd28) at /home/mrhuu/SharedFolder/_git/devilutionX/Source/engine.cpp:691
#3  dvl::mem_free_dbg (p=0x867cd28) at /home/mrhuu/SharedFolder/_git/devilutionX/Source/engine.cpp:687
#4  0x0016b6ec in dvl::(anonymous namespace)::SaveHelper::flush (this=0x8007608)
    at /home/mrhuu/SharedFolder/_git/devilutionX/Source/loadsave.cpp:174
#5  dvl::(anonymous namespace)::SaveHelper::~SaveHelper (this=<optimized out>, this=<optimized out>)
    at /home/mrhuu/SharedFolder/_git/devilutionX/Source/loadsave.cpp:180
#6  dvl::SaveHeroItems (pPlayer=0x4417a8 <plr>) at /home/mrhuu/SharedFolder/_git/devilutionX/Source/loadsave.cpp:1741
#7  0x001ad1b0 in dvl::pfile_ui_save_create (heroinfo=0x458340 <dvl::selhero_heroInfo>)
    at /home/mrhuu/SharedFolder/_git/devilutionX/Source/pfile.cpp:351
#8  0x00115888 in dvl::selhero_Name_Select (value=<optimized out>)
    at /home/mrhuu/SharedFolder/_git/devilutionX/SourceX/DiabloUI/selhero.cpp:429
#9  0x0010b308 in dvl::(anonymous namespace)::HandleMenuAction (menu_action=<optimized out>)
    at /home/mrhuu/SharedFolder/_git/devilutionX/SourceX/DiabloUI/diabloui.cpp:225
#10 0x0010bb98 in dvl::UiFocusNavigation (event=0x8007ca8)
    at /home/mrhuu/SharedFolder/_git/devilutionX/SourceX/DiabloUI/diabloui.cpp:279
warnwar commented 3 years ago

@AJenbo I have a New 3DS XL. I thought the original 3DS and the other lower-powered ones were incompatible.

I tried that build you suggested, and it also crashes in the same way. The game saves properly, but then the system turns off.

AJenbo commented 3 years ago

Kinda odd that this would fail, since it free is guarded by if (p != null) so I don't think this is a double-free :/

AJenbo commented 3 years ago

I tried explicitly calling flush(), could you see if this helps? CIA: https://24981-143324737-gh.circle-artifacts.com/0/devilutionx.cia 3DSX: https://24981-143324737-gh.circle-artifacts.com/0/devilutionx.3dsx

warnwar commented 3 years ago

That build is still crashing for me. I'm seeing the "Saving..." window, and then it momentarily flickers back into the game before I get the black screen of death.

If it helps any, here is a picture of the crash: crash

If a video or more crash dumps would help, I can add them.

AJenbo commented 3 years ago

@glebm do you think this is related to the issue you saw in SaveHelper?

AJenbo commented 3 years ago

@MrHuu any chance you have time to look in to this. would be really sad if it slips the release :(

MrHuu commented 3 years ago

Trying to bisect as we speak. Unfortunately, i'm running into multiple other issues with previous commits. Not sure where it went wrong yet.

glebm commented 3 years ago

I don't think so but I am curious to know if #1314 fixes it

glebm commented 3 years ago

@MrHuu I have just rebased #1314, can you please try a build from that CI run?

MrHuu commented 3 years ago

@MrHuu I have just rebased #1314, can you please try a build from that CI run?

The .3dsx build still seems to crash in a similar way.

I noticed some commits about audio streams not functioning properly. Could it possibly be related to this?

glebm commented 3 years ago

It shouldn't be, the build was working at some point after 88a68f503a34cce70982b15fa4da247ddffce9ba

warnwar commented 3 years ago

After testing over 10 different builds, I think I found the commits that caused this issue: Build 3894 works fine ( https://app.circleci.com/pipelines/github/diasurgical/devilutionX/3894/workflows/d5291751-6318-49c7-a15f-6615d0df2ad9/jobs/24231 ) Build 3896 does not work ( https://app.circleci.com/pipelines/github/diasurgical/devilutionX/3896/workflows/ae393c4c-61a3-4892-ad4b-171056cfac8a/jobs/24256/artifacts )

Hopefully this helps. I don't understand c++ well enough to investigate beyond this on the code side.

AJenbo commented 3 years ago

Looks like taking a shower before releasing payed off :D

AJenbo commented 3 years ago

Hmm the build you marked as broken isn't part of the main branch, I'm guessing that you did this going overbuilds in CircleCI. Would be best to have it filtered by branch.

Could you test this build as that is what actually got merged: https://app.circleci.com/pipelines/github/diasurgical/devilutionX/3904/workflows/5680ccce-2b9f-4146-b643-72a69260ea0f/jobs/24328/artifacts

We should also test bba73ea20f77777420a8c9756c1453a2028ef4cb as that is between the two, but doesn't have a CircelCI build. Let me know if you would like me to trigger a build for it in case it's easier for you then to build it locally.

warnwar commented 3 years ago

I knew I kinda messed it up comparing branch and master builds, still I think it was worth the effort. That build 3904 is broken. edit: didn't read all of your message before...

Feel free to start any other builds you would like me to test.

AJenbo commented 3 years ago

CircelCI build 3904 is a build of version 4082dee, it skipped bba73ea because both were submitted together. But looking at it I see that 4082dee just added a test so the two versions would have been identified so no need to test them both anyway.

I have submitted a change that might fix the issue, but it's still a bit of a shot in the dark since we don't know what's really going one here. Just where it crashes (the commit you located is the one where the function was added). But it does confirm that this is where we should try to solve things probably.

AJenbo commented 3 years ago

@warnwar can you test this: https://app.circleci.com/pipelines/github/diasurgical/devilutionX/4063/workflows/df24d881-35e3-4d9f-89aa-83472acec03d/jobs/25914/artifacts

AJenbo commented 3 years ago

After testing over 10 different builds, I think I found the commits that caused this issue: Hopefully this helps. I don't understand c++ well enough to investigate beyond this on the code side.

Wow, I thought that was @MrHuu replying :sweat_smile: I was confused why you were writing that you didn't understand C++. Sorry for that, thank you for tirelessly going over all the builds and testing, even figuring that out is pretty nice for someone who is new to all of this :)

warnwar commented 3 years ago

Always willing to help, and finally some good news: 64575d4 saves properly. I tried creating a new character and that saved fine. Then saving within the game also worked.

AJenbo commented 3 years ago

OMG, I love you :D This is tight timing. I was going to push the release and retract the 3DS port until the next cycle. Decided to take a shower to give anyone working on the 3DS issue figure out what might be wrong, and that was exactly enough for you to find the offending build so I could guess at the issue from a previous comment by @glebm :joy:

So good news, issue solved and 3DS will be an official release target :tada:

julealgon commented 3 years ago

@warnwar your willingness to help is hugely appreciated. Going as far as to narrowing it down was honestly unexpected. Kudos to you.