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

[core] Fix a data race #2984

Closed
wants to merge 5 commits into from
Closed

Conversation

yomnes0
Copy link
Collaborator

@yomnes0 yomnes0 commented Jul 16, 2024

Based on #PR2981
Add a shared mutex on m_pListener to avoid a data race condition.
This fixes #2970

Improved syntax
Changed lock/unlock functions name to match reference implementation
Add unit tests for SharedMutex
Add a CSharedObject template class that protects an object
@yomnes0 yomnes0 added Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core labels Jul 17, 2024
@maxsharabayko maxsharabayko added this to the v1.5.4 milestone Jul 29, 2024
@maxsharabayko
Copy link
Collaborator

To keep track, this PR misses the very data race fix originally done in #2961. The proposed fix was in locking m_ConnectionLock before creating a new socket. The blocker was the m_LSLock mutex that was causing lock order inversion.

    try
    {
+       ScopedLock col(ls->core().m_ConnectionLock);
        ns = new CUDTSocket(*ls);
        // No need to check the peer, this is the address from which the request has come.
        ns->m_PeerAddr = peer;

The CSharedObjectPtr and SharedLock are being added in PR #2996. As well as changing the type of the m_pListener to CSharedObjectPtr.

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 Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Lock order violation: m_ConnectionLock and m_LSLock.
2 participants