Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash fix of Gran Turismo on Quest 2 #16399

Closed
wants to merge 2 commits into from

Conversation

lvonasek
Copy link
Contributor

I wasn't able to figure out why is this happening but it seems the destructor of a texture is called twice.

This PR prevents that the second destruction causes a crash.

@unknownbrackets
Copy link
Collaborator

Is GLRenderManager::DeleteTexture() { being called twice in this scenario? You could temporarily add a field int deletes = 0; to GLRTexture, and increment it in void DeleteTexture(GLRTexture *texture) {, then break or log if it isn't zero on the call.

-[Unknown]

@lvonasek
Copy link
Contributor Author

Is GLRenderManager::DeleteTexture() { being called twice in this scenario? You could temporarily add a field int deletes = 0; to GLRTexture, and increment it in void DeleteTexture(GLRTexture *texture) {, then break or log if it isn't zero on the call.

-[Unknown]

I will check it and give you know. I cought the crash from here:

@lvonasek
Copy link
Contributor Author

lvonasek commented Nov 19, 2022

hmmm, it seems I was wrong. It crashes randomly, so it is hard to confirm the fix:

*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
Build fingerprint: 'oculus/hollywood/hollywood:10/QQ3A.200805.001/47421700959100000:user/release-keys'
Revision: '0'
ABI: 'arm64'
Timestamp: 2022-11-19 22:29:40+0100
pid: 4313, tid: 4370, name: AndroidRender  >>> org.ppsspp.ppsspp <<<
uid: 10354
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
Cause: null pointer dereference
    x0  0000007aea76f380  x1  0000007b09ba6dc0  x2  0000000000000074  x3  0000000000000000
    x4  0000000000000000  x5  0000007aeb8f33e0  x6  0000007aeb6a99b8  x7  0000007aeb6a99b8
    x8  0000000000000001  x9  0000000000000000  x10 0000007b09aa2b40  x11 0000007b71737373
    x12 0000007b5d51fe60  x13 0000007aeb6a9eb8  x14 0000000000003b00  x15 00000000fffeffff
    x16 0000000000000000  x17 0000000000000000  x18 0000007aef42c000  x19 0000007add6da540
    x20 0000007b09ba6dc0  x21 0000007aeb6a5e00  x22 0000000000000001  x23 0000007add6da540
    x24 0000000000000001  x25 0000007ad92bf100  x26 0000000000000000  x27 0000007add6da540
    x28 0000000000000001  x29 0000007aef6d1dd0
    sp  0000007aef6d1db0  lr  0000007b5d37293c  pc  0000000000000000
backtrace:
      #00 pc 0000000000000000  <unknown>
      #01 pc 00000000002b0938  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!bb42a6688242c365a0a7febd2e17e4!f9286358ab9!+144) (BuildId: d276693e032a36a839eb547ca0b029f9)
      #02 pc 0000000000285da0  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!28f24188499da127c115ae7e78c710!f9286358ab9!+536) (BuildId: d276693e032a36a839eb547ca0b029f9)
      #03 pc 00000000002111a4  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!58cbc5b86fa13da0d4e51c7edcc648!f9286358ab9!+548) (BuildId: d276693e032a36a839eb547ca0b029f9)
      #04 pc 000000000071ab90  /data/app/org.ppsspp.ppsspp-8ynCBbv8lYPw3foXwBxu4A==/lib/arm64/libppsspp_jni.so (GLDeleter::Perform(GLRenderManager*, bool)+592) (BuildId: 5702bbab1332481c5fa7d905120250dca37fff6e)
      #05 pc 000000000071b968  /data/app/org.ppsspp.ppsspp-8ynCBbv8lYPw3foXwBxu4A==/lib/arm64/libppsspp_jni.so (GLRenderManager::ThreadFrame()+244) (BuildId: 5702bbab1332481c5fa7d905120250dca37fff6e)
      #06 pc 00000000007675d0  /data/app/org.ppsspp.ppsspp-8ynCBbv8lYPw3foXwBxu4A==/lib/arm64/libppsspp_jni.so (Java_org_ppsspp_ppsspp_NativeRenderer_displayRender+104) (BuildId: 5702bbab1332481c5fa7d905120250dca37fff6e)
      #07 pc 000000000000908c  /data/app/org.ppsspp.ppsspp-8ynCBbv8lYPw3foXwBxu4A==/oat/arm64/base.odex (art_jni_trampoline+124)
      #08 pc 0000000002001fb0  /memfd:/jit-cache (deleted)

Gran Turismo creates/destroys per frame a lot of textures. It could be something wrong with the driver.

@hrydgard
Copy link
Owner

Hm, I do think it's more likely we have a bug. Suspicious.

@hrydgard hrydgard modified the milestone: v1.14.0 Nov 19, 2022
@lvonasek lvonasek marked this pull request as draft November 19, 2022 22:42
@unknownbrackets
Copy link
Collaborator

Another thing worth trying if the above doesn't bare fruit: leak texture names. We don't currently use sized textures, so you can essentially delete a texture's storage using glTexImage2D() with null values. Then just don't glDeleteTextures() it. If it runs multiple times without crashing that way, then I'd guess some kind of use-after-free...

-[Unknown]

@lvonasek
Copy link
Contributor Author

Another thing worth trying if the above doesn't bare fruit: leak texture names. We don't currently use sized textures, so you can essentially delete a texture's storage using glTexImage2D() with null values. Then just don't glDeleteTextures() it. If it runs multiple times without crashing that way, then I'd guess some kind of use-after-free...

-[Unknown]

I didn't completely get your idea, however it brought me to 4c48361, where I spend 30 minutes of testing and it didn't crash. It seems calling glGenTextures, glDestroyTextures on Quest 2 makes some troubles if it is called too often (at least once per frame in Grand Turismo).

Do you think this could get merged after adding a teardown?

@lvonasek lvonasek marked this pull request as ready for review November 20, 2022 12:55
@lvonasek lvonasek changed the title Crash fix of Grand Turismo on Quest 2 Crash fix of Gran Turismo on Quest 2 Nov 26, 2022
@hrydgard hrydgard added this to the v1.14.0 milestone Nov 26, 2022
@lvonasek lvonasek mentioned this pull request Dec 3, 2022
11 tasks
@hrydgard
Copy link
Owner

hrydgard commented Dec 3, 2022

I'm thinking about an alternative where we only recycle small textures (like GT:s text characters) and leave the rest as before. I'll give it a shot in a bit (a modified version of 4c48361)

hrydgard added a commit that referenced this pull request Dec 4, 2022
Will hopefully work as a cleaner replacement for #16399.
@hrydgard
Copy link
Owner

hrydgard commented Dec 4, 2022

Alright, implemented that in #16498. If that works well, I want to replace this PR with that.

hrydgard added a commit that referenced this pull request Dec 4, 2022
Will hopefully work as a cleaner replacement for #16399.
@hrydgard
Copy link
Owner

hrydgard commented Dec 4, 2022

Updated #16498 with some more changes.

@unknownbrackets
Copy link
Collaborator

Is it possible this crash isn't happening anymore after #16533?

-[Unknown]

@lvonasek
Copy link
Contributor Author

I confirm that the crashing does not happen in the master anymore.

@lvonasek lvonasek closed this Dec 10, 2022
@lvonasek lvonasek deleted the hotfix_gt_crash branch December 10, 2022 09:35
@hrydgard
Copy link
Owner

Great, thanks for testing!

@unknownbrackets unknownbrackets removed this from the v1.14.0 milestone Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants