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

Allow a clean shutdown on window system failure #14625

Merged
merged 4 commits into from
Oct 19, 2018

Conversation

lrusak
Copy link
Contributor

@lrusak lrusak commented Oct 16, 2018

I have been investigating why we segfault when windowing cannot be initialized.

This allows a clean shutdown if false is return somewhere in InitWinSystem()

If this isn't desired it would still be nice to add the nullptr checks

This work is from investigating why we segfault when failing
to initialize windowing. Basically nothing is cleaned up properly
and some components were nullptr when accessed
@lrusak lrusak added Type: Fix non-breaking change which fixes an issue v18 Leia labels Oct 16, 2018
@lrusak lrusak added this to the Leia 18.0-beta4 milestone Oct 16, 2018
@FernetMenta
Copy link
Contributor

nice. I misinterpreted the intent of this code in the morning. This looks good

@lrusak
Copy link
Contributor Author

lrusak commented Oct 16, 2018

@FernetMenta thank you. Please note that the GLES renderer changes were do to some odd behaviour in the way std::unique_ptr<CGLESShader*[]> m_pShader; was working with nullptr checks.

It seems that

if (m_pShader[SM_DEFAULT])

was passing the nullptr check for some reason and this would cause a segfault in the Free()

I'm not sure why this was or if someone can explain this behaviour to me. Regardless I think the std::array<std::unique_ptr<CGLESShader>, SM_MAX> m_pShader; format looks better anyways.

If you want me to do this to GL just say so unless you want to do it 😉

@FernetMenta
Copy link
Contributor

@lrusak I noticed this problem too as I did: #14612
If ReleaseShaders was called before InitShaders it crashed. I came to this solution with an init member: 1d06fd4
Your solution seems to be more elegant. Please go ahead if you have time.

@lrusak
Copy link
Contributor Author

lrusak commented Oct 17, 2018

@FernetMenta care to give this a test before merging?

@MartijnKaijser
Copy link
Member

jenkins build this please

@FernetMenta
Copy link
Contributor

I tested the GL commit here: works

@lrusak lrusak merged commit a3f5e1e into xbmc:master Oct 19, 2018
@lrusak lrusak deleted the clean-shutdown branch February 11, 2022 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants