Introducing sync Mutex wrapper class #1064
Conversation
maxsharabayko
added
Priority: High
Type: Maintenance
and m_PassCond to m_BufferLock and m_BufferCond
maxsharabayko
force-pushed
the
develop/sync_02_mutexes
branch
3 times, most recently
from
d5f98b5 to
54b17c2
Compare
The only place affected is the garbage collector thread. On Linux it used to wait for 10 us, on Windows - 1 millisecond. Now 1 millisecond on both platforms.
maxsharabayko
force-pushed
the
develop/sync_02_mutexes
branch
2 times, most recently
from
1315d06 to
84c2110
Compare
| THREAD_RESUMED(); | ||
| } | ||
| } | ||
| CGuard::leaveCS(self->m_RecvLock); | ||
| recv_lock.unlock(); |
There was a problem hiding this comment.
Unnecessary. Can be unlocked in destructor here.
There was a problem hiding this comment.
I think this unlock is done to have the following section out of the guard:
THREAD_EXIT();
HLOGC(tslog.Debug, log << self->CONID() << "tsbpd: EXITING");
There was a problem hiding this comment.
So, no. This THREAD_EXIT() is an entry for Makito thread tracer stuff, it should actually be at the very end of this function. The logging instruction is also alright to be under or outside the CS.
There was a problem hiding this comment.
You are probably right on the usage.
Still, I would like to retain the previous behavior for convenience. And previously the mutex was being explicitly unlocked.
There was a problem hiding this comment.
Yes, but it was intended to be unlocked at the end of the function. Previously, before I added the logging instruction, it was really at the end, only followed by THREAD_EXIT(). I simply added this log by jumping to the end of this function, ignoring whatever was there before. I confirm officially: it wasn't my intention to have the mutex unlocked specifically before the logging instruction. Also, the original author of this code simply didn't use CGuard.
| { | ||
| // Use class member such that the method can be interrupted by others | ||
| m_tsSchedTime = nexttime; | ||
| // Use class member such that the method can be interrupted by others |
There was a problem hiding this comment.
Better leave formatting fixes behind, if possible.
srtcore/sync.cpp
Outdated
|
|
||
| bool srt::sync::Mutex::try_lock() | ||
| { | ||
| return (pthread_mutex_lock(&m_mutex) == 0); |
There was a problem hiding this comment.
try_lock() should be using pthread_mutex_trylock() or be removed to avoid confusion.
maxsharabayko
force-pushed
the
develop/sync_02_mutexes
branch
from
84c2110 to
53069ba
Compare
This is the second subset of PR #800 on the way to C++11 support.
srt::sync::Mutexclass is introduced. The class wraps pthread mutex.In future it is expected to be directly mapped to
std::mutexin case SRT is built with C++11 threads.Shortlist of Changes
srt::sync::Mutexclass that wraps pthread mutex.CGuardwithScopedLockandUniqueLock. Added CGuard typedef to reduce the number of renaming until Added CSync utlity for CV #1049 is merged.CRcvQueue::m_PassLocktoCRcvQueue::m_BufferLockCRcvQueue::m_PassCondtoCRcvQueue::m_BufferCondCTimer::sleep()bysrt::sync::SleepFor(const Duration&)Blocks #1049