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

UI: Fix wrong order for sources_mutex #11687

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

Conversation

walker-WSH
Copy link
Contributor

@walker-WSH walker-WSH commented Jan 1, 2025

Description

Below callstack is from graphic thread where gs_enter has been gotten. As this, the order of requesting mutex is as gs->sources_mutex. That is conflicting with basic logic in libobs. About basic request order, sources_mutex should be requested before gs.

[callstask]
render_displays
(1)request gs enter
DrawSpacingHelpers()
CreateLabel
obs_source_create_private
(2)request obs->data.sources_mutex

Motivation and Context

keep the stable order for requesting mutex. otherwise there will be a nested logic which will cause block.
But what I must declare is that this block is not easily happen

How Has This Been Tested?

run obs and select any video source,
ensure pixel label in preview window can be displayed normally

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.

Below callstack is from graphic thread where gs_enter has been gotten.
As this, the order of requesting mutex is as gs->sources_mutex. That
is conflicting with basic logic in libobs. About basic request order,
sources_mutex should be requested before gs.

[callstask]
render_displays
(1)request gs enter
DrawSpacingHelpers()
CreateLabel
obs_source_create_private
(2)request obs->data.sources_mutex
@walker-WSH walker-WSH changed the title UI: Fix wrong order for requesting mutex UI: Fix wrong order for sources_mutex Jan 1, 2025
@WizardCM WizardCM added the Bug Fix Non-breaking change which fixes an issue label Jan 4, 2025
Copy link
Collaborator

@Lain-B Lain-B left a comment

Choose a reason for hiding this comment

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

Question: You say that it's in conflict with libobs logic. Where does the sources_mutex get locked before the graphics mutex in libobs?

Comment: An alternative way to handle this would be to exit the graphics mutex before the label creation, and then enter it again after. That should be safe to do as well, although that code might be equally awkward. (Nevermind, this alternative I made would be bad, ignore this part please)

@walker-WSH
Copy link
Contributor Author

hi @Lain-B

(1) in obs_load_sources, sources_mutex will firstly be requested, then create all sources.
in obs_source_info::create function, gs may be requested since gs_texture need to be created in any plugins.

(2) in the past, tick_sources function will call tick function for all sources and gs may be requested in any tick function to update texture.
surely, now tick_sources has no this case since tick function is running outside of sources_mutex

@walker-WSH
Copy link
Contributor Author

walker-WSH commented Jan 5, 2025

another method:

create label source early, not late in display callback

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants