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] Fixed cycle-deadlock on m_ConnectionLock and refax on locks in SndUList #1844

Closed
wants to merge 4 commits into from

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Mar 5, 2021

Changes:

  1. Refactored locks in SndUList and SndQueue. Actually m_WindowLock was also locking resources for SndUList and "borrowing" them from SndQueue was a poor design. Instead SndQueue is given limited access to these locks and the locks are contained in the SndUList, which should clear the controversies for using two different locks for the same purpose.
  2. Also changed the lock form into CEvent as the lock serves also the purpose of locking around a CV.
  3. Added temporary unlocking of mutexes that would be locked in the wrong order.
  4. Removed the lock for CUDTSocket::m_ControlLock that breaks the ordering against m_ConnectionLock. This lock is likely not necessary provided that m_GlobControlLock is also applied.

Likely replaced by #2743

…onnectionLock and refax on locks in SndULust
@maxsharabayko
Copy link
Collaborator

Reviewing notes

  • CSndUList - list (heap) of CUDT sockets to send data packets.

    • m_ListLock - locks CSndUList modifications. update, pop, remove, getNextProcTime.
    • m_pWindowLock - pointer to mutex owned by CSndQueue, only used together with the CV to signal there was a new entry added if it was empty.
    • m_pWindowCond - a pointer to a CV owned by CSndQueue.
  • CSndQueue - a class that owns CSndUList.

    • m_WindowLock
    • m_WindowCond - CV used to wait for CSndUList to become non-empty. Also signalled in the destructor to unblock the worker thread.

@ethouris
Copy link
Collaborator Author

Closed.

  1. One fix is provided different way
  2. The other is left in [core] Fixing some lock-order-inversion and data race problems (TSAN reports) #1824.
  3. Another fix is proven not necessary and developer info about this is updated.

@ethouris ethouris closed this Jan 19, 2024
@maxsharabayko maxsharabayko modified the milestones: Backlog, v1.5.4 Jan 19, 2024
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.

2 participants