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

Don't loop CEF exit while shutting down if task post fails #476

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tt2468
Copy link
Member

@tt2468 tt2468 commented Feb 19, 2025

Description

Removes a while loop which loops infinitely if posting the shutdown task to the manager thread fails. In scenarios where Cef failed to initialize, this manager thread exits early, leaving a situation where tasks cannot be posted to it. In normal scenarios, posting the task is not expected to fail anyway.

This also logs a debug message just in case it becomes relevant in the future.

Motivation and Context

If the manager_thread exits early, there will be no message loop to post the task to, and a permanent deadlock would occur.

Rather than guaranteeing a deadlock, this at least lowers the odds of one.

How Has This Been Tested?

  • Ubuntu 24.04 - X11 on NVIDIA
  • Tested a standard working configuration where browsers load and then OBS is closed gracefully
  • Tested a non-working configuration where CEF fails to initialize due to an invalid SingletonCookie

All test scenarios exited gracefully without deadlocking.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

If the `manager_thread` exits early, there will be no message loop to
post the task to, and a permanent deadlock would occur. Rather than
guaranteeing a deadlock, this at least lowers the odds of one.

A future implementation might be more aware of the run state of the
manager thread and handle these exit scenarios more explicitly.
@WizardCM WizardCM added Bug Fix Non-breaking change which fixes an issue Seeking Testers labels Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue Seeking Testers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants