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

Fixed:Headset-client-crashing-on-second-startup #1783

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

ydi-mua
Copy link
Contributor

@ydi-mua ydi-mua commented Aug 12, 2023

Headset client crashing on startup #1610
If I restart my headset, it consistently crashes once, then opens successfully on the second attempt. If i quit the app (and by quit I mean press the oculus button, then press the quit button on the window that pops up), and open it again, it crashes. Open it right after the crash and it works. Quit, open and crash, open no crash, quit, open and crash, open no crash... And so on.

zmerp
zmerp previously requested changes Aug 12, 2023
Copy link
Member

@zmerp zmerp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR has been rebased wrong. The release_android_context() call must be done inside destroy(), not alvr_destroy().

@ydi-mua
Copy link
Contributor Author

ydi-mua commented Aug 12, 2023

I think this PR has been rebased wrong. The release_android_context() call must be done inside destroy(), not alvr_destroy().

but the initialize_android_context() in alvr_initialize call.
image

@zmerp
Copy link
Member

zmerp commented Aug 12, 2023

Hm ok. Have you experienced the same type of crash on the release v20.0.0 or v20.1.0?

@ydi-mua
Copy link
Contributor Author

ydi-mua commented Aug 12, 2023

Hm ok. Have you experienced the same type of crash on the release v20.0.0 or v20.1.0?

In 19.1.1, not tested on version 20+.

@zmerp
Copy link
Member

zmerp commented Aug 12, 2023

release_android _context() seems to be already called by android-activity, which manages the entry point on the new openxr app (since v20). So your PR is still correct, but the code will never be hit on the ALVR client, only for other apps using the ALVR C interface, like PhoneVR.

@ydi-mua
Copy link
Contributor Author

ydi-mua commented Aug 12, 2023

release_android _context() seems to be already called by android-activity, which manages the entry point on the new openxr app (since v20). So your PR is still correct, but the code will never be hit on the ALVR client, only for other apps using the ALVR C interface, like PhoneVR.

Okay

@ShootingKing-AM
Copy link
Member

ShootingKing-AM commented Aug 16, 2023

Tested on v20.1.0, this pr paritally solves the crash on PhoneVR (thanks @ydi-mua), but still there is some GLThread Crash, after restarting/warm starting (maybe not related to code on this repo)

10:15:55.016 12710-12943 [ALVR NATIVE-RUST]      viritualisres.phonevr                I  [ALVR NATIVE] renderingParamsChanged, sending new params to alvr
10:15:55.018 12710-12943 libc                    viritualisres.phonevr                A  Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0xffffffff80000008 in tid 12943 (GLThread 56), pid 12710 (alisres.phonevr)

related issue: alvr-org/PhoneVR#129

@zmerp
Copy link
Member

zmerp commented Aug 16, 2023

@ShootingKing-AM This is a long standing bug, i never put the time to fix it, so if this was fixed on upstream ALVR it was purely by accident.

@zmerp zmerp dismissed their stale review August 16, 2023 05:46

It is actually correct as it is

@zmerp
Copy link
Member

zmerp commented Aug 16, 2023

In any case I think this PR can be merged

@zmerp zmerp merged commit f650c5d into alvr-org:master Aug 16, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants