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

[BUG][core] Unguarded access to epolldesc from group sender code #1846

Merged

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Mar 5, 2021

The problem detected by thread sanitizer: The calls to m_SndEPolldesc->watch_empty() is done without locking, while all operations on epoll require a lock on CEPoll::m_EPollLock.

Changes:

  • Made whole CEPollDesc class private with exclusive access for CEPoll. This is to ensure that the CEPollDesc class isn't used "privately" by any other part of the code.
  • Added an access function CEPoll::empty to return information about having no subscribers, with appropriate locking.

@ethouris ethouris added Priority: High Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core labels Mar 5, 2021
@maxsharabayko maxsharabayko added this to the v1.4.3 milestone Mar 8, 2021
Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

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

Minor suggestion. 👇

srtcore/epoll.cpp Show resolved Hide resolved
@ethouris
Copy link
Collaborator Author

ethouris commented Mar 8, 2021

Sorry I had to withdraw this suggestion. This is wrong - although the function is in general read-only, locking the mutex is a writable operation, as per class. Might be that the mutex can be made itself mutable, but this would go too far as a refax and better be postponed.

@maxsharabayko maxsharabayko merged commit 47e477b into Haivision:master Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Priority: High Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants