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

Fix OpenAL-Soft crash on exit #205

Closed
wants to merge 1 commit into from
Closed

Fix OpenAL-Soft crash on exit #205

wants to merge 1 commit into from

Conversation

matpow2
Copy link

@matpow2 matpow2 commented Apr 12, 2012

Fix issue #30 by removing AudioDevice's crashing deconstructor (OpenAL-Soft cleans up anyway)

@LaurentGomila
Copy link
Member

Do you really expect me to merge this commit?

@matpow2
Copy link
Author

matpow2 commented Apr 12, 2012

Yes. You should not use an atexit deconstructor to deinitialize the OpenAL context because OpenAL-Soft already does so:
http://repo.or.cz/w/openal-soft.git/blob/HEAD:/Alc/ALc.c#l466
This will lead to crashes on exit on Windows XP 32bit SP3. If you don't want to let OpenAL-Soft deinitialize the context by itself, you should add a public deinitialize method so we can tear down the context before atexit.

@LaurentGomila
Copy link
Member

This is not a clean solution. A correct OpenAL program must call the cleanup functions corresponding to the init functions that it called at startup.

Relying on a internal/hidden undocumented "feature" is not an option.

@matpow2
Copy link
Author

matpow2 commented Apr 12, 2012

In any case, the atexit SFML AudioDevice deconstructor always crashes the program on exit (at least on Windows XP - it is most likely the case for Vista/7 too, as per #30), and to me, that's not an option, and not very clean either ;-)

If you add a public deinitialize function for the OpenAL context so we can destroy the context before atexit, that would solve the problem also. I didn't want to add a function to the API when this fix would be compatible with existing code.

Those are the two solutions I've thought about. How would you fix it otherwise?

@LaurentGomila
Copy link
Member

I never meant that the current solution was acceptable -- that's why issue #30 exists.

But I won't implement a quick and dirty solution. I'll take some time to think about it, and hopefully find a clean and robust solution.

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.

None yet

2 participants