Skip to content

Delay fStreamListMutex initialization #33

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

Closed
wants to merge 2 commits into from
Closed

Conversation

glebm
Copy link
Contributor

@glebm glebm commented Jun 22, 2022

Initializes the mutex on init and reports errors in the same way as we do for all other errors (via SDL_GetError()).

This also eliminates the only use of exceptions in SDL_audiolib.

`std::optional` fallback to `std::experimental::optional` for older
compilers.
Initializes the mutex on `init` and reports errors in the same way as we
do for all other errors (via `SDL_GetError()`).

This also eliminates the only use of exceptions in SDL_audiolib.
@realnc
Copy link
Owner

realnc commented Jun 22, 2022

Sorry, no :P Locking is not optional. If a lock cannot be acquired, the application should shut down since something is seriously broken. The exception that is thrown simply gives applications the opportunity to do a more graceful shutdown. It's not a condition that can just be logged and then have execution resumed.

On systems where you build without exceptions, I think it should probably just terminate.

Let's go back to #32 and simply sort it out there.

@glebm
Copy link
Contributor Author

glebm commented Jun 22, 2022

The mutex is initialized statically, the application has no chance to catch this exception at all even if exceptions are enabled.

Perhaps just replace with std::abort if you feel strongly that the application should not be able to continue?

@realnc
Copy link
Owner

realnc commented Jun 22, 2022

Closing in favor of #32.

@realnc realnc closed this Jun 22, 2022
@realnc
Copy link
Owner

realnc commented Jun 22, 2022

The mutex is initialized statically, the application has no chance to catch this exception at all even if exceptions are enabled.

Thanks for pointing that out. I'll need to change it.

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.

2 participants