diff --git a/CMakeLists.txt b/CMakeLists.txt index 9a2fc63d2..e5bc7e3bb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -850,7 +850,7 @@ if (ENABLE_THREAD_CHECK) endif() if (ENABLE_CLANG_TSA) - list(APPEND SRT_EXTRA_CFLAGS "-Wthread-safety") + list(APPEND SRT_EXTRA_CFLAGS "-Wthread-safety -Wthread-safety-beta") message(STATUS "Clang TSA: Enabled") endif() diff --git a/srtcore/api.cpp b/srtcore/api.cpp index bb5dd64fe..6906c3ef7 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -99,6 +99,7 @@ srt::CUDTSocket::~CUDTSocket() releaseMutex(m_ControlLock); } +SRT_TSA_DISABLED // Uses m_Status that should be guarded, but for reading it is enough to be atomic SRT_SOCKSTATUS srt::CUDTSocket::getStatus() { // TTL in CRendezvousQueue::updateConnStatus() will set m_bConnecting to false. @@ -128,6 +129,7 @@ void srt::CUDTSocket::breakSocket_LOCKED() setClosed(); } +SRT_TSA_DISABLED // Uses m_Status that should be guarded, but for reading it is enough to be atomic void srt::CUDTSocket::setClosed() { m_Status = SRTS_CLOSED; @@ -2340,7 +2342,7 @@ int srt::CUDTUnited::selectEx(const vector& fds, if (readfds) { - if ((s->core().m_bConnected && s->core().m_pRcvBuffer->isRcvDataReady()) || + if ((s->core().m_bConnected && s->core().isRcvBufferReady()) || (s->core().m_bListening && (s->m_QueuedSockets.size() > 0))) { readfds->push_back(s->m_SocketID); @@ -3508,12 +3510,12 @@ srt::CUDT::APIError::APIError(CodeMajor mj, CodeMinor mn, int syserr) // This doesn't have argument of GroupType due to header file conflicts. // [[using locked(s_UDTUnited.m_GlobControlLock)]] -srt::CUDTGroup& srt::CUDT::newGroup(const int type) +srt::CUDTGroup& srt::CUDTUnited::newGroup(const int type) { - const SRTSOCKET id = uglobal().generateSocketID(true); + const SRTSOCKET id = generateSocketID(true); // Now map the group - return uglobal().addGroup(id, SRT_GROUP_TYPE(type)).set_id(id); + return addGroup(id, SRT_GROUP_TYPE(type)).set_id(id); } SRTSOCKET srt::CUDT::createGroup(SRT_GROUP_TYPE gt) @@ -3524,7 +3526,7 @@ SRTSOCKET srt::CUDT::createGroup(SRT_GROUP_TYPE gt) try { srt::sync::ScopedLock globlock(uglobal().m_GlobControlLock); - return newGroup(gt).id(); + return uglobal().newGroup(gt).id(); // Note: potentially, after this function exits, the group // could be deleted, immediately, from a separate thread (tho // unlikely because the other thread would need some handle to @@ -4391,16 +4393,6 @@ srt::CUDT* srt::CUDT::getUDTHandle(SRTSOCKET u) } } -vector srt::CUDT::existingSockets() -{ - vector out; - for (CUDTUnited::sockets_t::iterator i = uglobal().m_Sockets.begin(); i != uglobal().m_Sockets.end(); ++i) - { - out.push_back(i->first); - } - return out; -} - SRT_SOCKSTATUS srt::CUDT::getsockstate(SRTSOCKET u) { try diff --git a/srtcore/api.h b/srtcore/api.h index b5d6be915..582ba8aa7 100644 --- a/srtcore/api.h +++ b/srtcore/api.h @@ -135,7 +135,10 @@ class CUDTSocket } - SRT_ATTR_GUARDED_BY(m_ControlLock) + // Controversial whether it should stand. This lock is mainly + // for API things connected to this socket, while status is also + // set as atomic to allow multi-thread access. + // SRT_TSA_GUARDED_BY(m_ControlLock) sync::atomic m_Status; //< current socket state /// Time when the socket is closed. @@ -269,6 +272,13 @@ class CUDTUnited /// @return The new UDT socket ID, or INVALID_SOCK. SRTSOCKET newSocket(CUDTSocket** pps = NULL); +#if ENABLE_BONDING + // This is an internal function; 'type' should be pre-checked if it has a correct value. + // This doesn't have argument of GroupType due to header file conflicts. + + SRT_TSA_NEEDS_LOCKED(m_GlobControlLock) + srt::CUDTGroup& newGroup(const int type); +#endif /// Create (listener-side) a new socket associated with the incoming connection request. /// @param [in] listen the listening socket ID. /// @param [in] peer peer address. @@ -337,7 +347,8 @@ class CUDTUnited int epoll_release(const int eid); #if ENABLE_BONDING - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_GlobControlLock) + SRT_ATR_NODISCARD + SRT_TSA_NEEDS_LOCKED(m_GlobControlLock) CUDTGroup& addGroup(SRTSOCKET id, SRT_GROUP_TYPE type) { // This only ensures that the element exists. @@ -356,10 +367,14 @@ class CUDTUnited return *g; } + SRT_TSA_NEEDS_NONLOCKED(m_GlobControlLock) void deleteGroup(CUDTGroup* g); + + SRT_TSA_NEEDS_LOCKED(m_GlobControlLock) void deleteGroup_LOCKED(CUDTGroup* g); - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_GlobControlLock) + SRT_ATR_NODISCARD + SRT_TSA_NEEDS_LOCKED(m_GlobControlLock) CUDTGroup* findPeerGroup_LOCKED(SRTSOCKET peergroup) { for (groups_t::iterator i = m_Groups.begin(); i != m_Groups.end(); ++i) @@ -398,15 +413,19 @@ class CUDTUnited private: typedef std::map sockets_t; // stores all the socket structures - SRT_ATTR_GUARDED_BY(m_GlobControlLock) + SRT_TSA_GUARDED_BY(m_GlobControlLock) sockets_t m_Sockets; #if ENABLE_BONDING typedef std::map groups_t; - SRT_ATTR_GUARDED_BY(m_GlobControlLock) + SRT_TSA_GUARDED_BY(m_GlobControlLock) groups_t m_Groups; #endif + // XXX Desired, but blocked because the older clang compilers + // do not handle this declaration correctly. Unblock in devel builds + // for checking. + // SRT_TSA_LOCK_ORDERS_AFTER(CUDT::m_ConnectionLock) sync::Mutex m_GlobControlLock; // used to synchronize UDT API sync::Mutex m_IDLock; // used to synchronize ID generation @@ -414,22 +433,27 @@ class CUDTUnited SRTSOCKET m_SocketIDGenerator; // seed to generate a new unique socket ID SRTSOCKET m_SocketIDGenerator_init; // Keeps track of the very first one - SRT_ATTR_GUARDED_BY(m_GlobControlLock) + SRT_TSA_GUARDED_BY(m_GlobControlLock) std::map > m_PeerRec; // record sockets from peers to avoid repeated connection request, int64_t = (socker_id << 30) + isn private: friend struct FLookupSocketWithEvent_LOCKED; + SRT_TSA_NEEDS_NONLOCKED(m_GlobControlLock) CUDTSocket* locateSocket(SRTSOCKET u, ErrorHandling erh = ERH_RETURN); // This function does the same as locateSocket, except that: // - lock on m_GlobControlLock is expected (so that you don't unlock between finding and using) // - only return NULL if not found + SRT_TSA_NEEDS_LOCKED(m_GlobControlLock) CUDTSocket* locateSocket_LOCKED(SRTSOCKET u); CUDTSocket* locatePeer(const sockaddr_any& peer, const SRTSOCKET id, int32_t isn); #if ENABLE_BONDING + SRT_TSA_NEEDS_NONLOCKED(m_GlobControlLock) CUDTGroup* locateAcquireGroup(SRTSOCKET u, ErrorHandling erh = ERH_RETURN); + + SRT_TSA_NEEDS_NONLOCKED(m_GlobControlLock) CUDTGroup* acquireSocketsGroup(CUDTSocket* s); struct GroupKeeper @@ -520,37 +544,38 @@ class CUDTUnited const sockaddr_any& reqaddr, const CSrtMuxerConfig& cfgSocket); private: - SRT_ATTR_GUARDED_BY(m_GlobControlLock) + SRT_TSA_GUARDED_BY(m_GlobControlLock) std::map m_mMultiplexer; // UDP multiplexer /// UDT network information cache. /// Existence is guarded by m_GlobControlLock, but the cache itself is thread-safe. - SRT_ATTR_GUARDED_BY(m_GlobControlLock) + SRT_TSA_PT_GUARDED_BY(m_GlobControlLock) CCache* const m_pCache; private: - srt::sync::atomic m_bClosing; + sync::atomic m_bClosing; sync::Mutex m_GCStopLock; sync::Condition m_GCStopCond; sync::Mutex m_InitLock; - SRT_ATTR_GUARDED_BY(m_InitLock) + SRT_TSA_GUARDED_BY(m_InitLock) int m_iInstanceCount; // number of startup() called by application - SRT_ATTR_GUARDED_BY(m_InitLock) - bool m_bGCStatus; // if the GC thread is working (true) + sync::atomic m_bGCStatus; // if the GC thread is working (true) - SRT_ATTR_GUARDED_BY(m_InitLock) + SRT_TSA_GUARDED_BY(m_InitLock) sync::CThread m_GCThread; static void* garbageCollect(void*); - SRT_ATTR_GUARDED_BY(m_GlobControlLock) + SRT_TSA_GUARDED_BY(m_GlobControlLock) sockets_t m_ClosedSockets; // temporarily store closed sockets #if ENABLE_BONDING - SRT_ATTR_GUARDED_BY(m_GlobControlLock) + SRT_TSA_GUARDED_BY(m_GlobControlLock) groups_t m_ClosedGroups; #endif void checkBrokenSockets(); + + SRT_TSA_NEEDS_LOCKED(m_GlobControlLock) void removeSocket(const SRTSOCKET u); CEPoll m_EPoll; // handling epoll data structures and events diff --git a/srtcore/buffer_rcv.cpp b/srtcore/buffer_rcv.cpp index 363bd7e9c..b7d289baa 100644 --- a/srtcore/buffer_rcv.cpp +++ b/srtcore/buffer_rcv.cpp @@ -218,9 +218,10 @@ std::pair CRcvBuffer::dropUpTo(int32_t seqno) return std::make_pair(0, 0); } - m_iMaxPosOff -= len; - if (m_iMaxPosOff < 0) - m_iMaxPosOff = 0; + int newmax = m_iMaxPosOff - len; + if (newmax < 0) + newmax = 0; + m_iMaxPosOff = newmax; int iNumDropped = 0; // Number of dropped packets that were missing. int iNumDiscarded = 0; // The number of dropped packets that existed in the buffer. @@ -776,7 +777,7 @@ void CRcvBuffer::countBytes(int pkts, int bytes) if (!m_uAvgPayloadSz) m_uAvgPayloadSz = bytes; else - m_uAvgPayloadSz = avg_iir<100>(m_uAvgPayloadSz, (unsigned) bytes); + m_uAvgPayloadSz = avg_iir<100, unsigned>(m_uAvgPayloadSz, bytes); } } diff --git a/srtcore/buffer_rcv.h b/srtcore/buffer_rcv.h index c5fca428b..eb3385ea6 100644 --- a/srtcore/buffer_rcv.h +++ b/srtcore/buffer_rcv.h @@ -358,10 +358,15 @@ class CRcvBuffer const size_t m_szSize; // size of the array of units (buffer) CUnitQueue* m_pUnitQueue; // the shared unit queue - int m_iStartSeqNo; + // ATOMIC because getStartSeqNo() may be called from other thread + // than CUDT's receiver worker thread. Even if it's not a problem + // if this value is a bit outdated, it must be read solid. + sync::atomic m_iStartSeqNo; int m_iStartPos; // the head position for I/O (inclusive) int m_iFirstNonreadPos; // First position that can't be read (<= m_iLastAckPos) - int m_iMaxPosOff; // the furthest data position + + // ATOMIC: sometimes this value is checked for buffer emptiness + sync::atomic m_iMaxPosOff; // the furthest data position int m_iNotch; // the starting read point of the first unit size_t m_numOutOfOrderPackets; // The number of stored packets with "inorder" flag set to false @@ -408,7 +413,7 @@ class CRcvBuffer mutable sync::Mutex m_BytesCountLock; // used to protect counters operations int m_iBytesCount; // Number of payload bytes in the buffer int m_iPktsCount; // Number of payload bytes in the buffer - unsigned m_uAvgPayloadSz; // Average payload size for dropped bytes estimation + sync::atomic m_uAvgPayloadSz; // Average payload size for dropped bytes estimation }; } // namespace srt diff --git a/srtcore/buffer_snd.h b/srtcore/buffer_snd.h index afd52110b..f2bec33d8 100644 --- a/srtcore/buffer_snd.h +++ b/srtcore/buffer_snd.h @@ -106,14 +106,14 @@ class CSndBuffer /// @param [in] data pointer to the user data block. /// @param [in] len size of the block. /// @param [inout] w_mctrl Message control data - SRT_ATTR_EXCLUDES(m_BufLock) + SRT_TSA_NEEDS_NONLOCKED(m_BufLock) void addBuffer(const char* data, int len, SRT_MSGCTRL& w_mctrl); /// Read a block of data from file and insert it into the sending list. /// @param [in] ifs input file stream. /// @param [in] len size of the block. /// @return actual size of data added from the file. - SRT_ATTR_EXCLUDES(m_BufLock) + SRT_TSA_NEEDS_NONLOCKED(m_BufLock) int addBufferFromFile(std::fstream& ifs, int len); // Special values that can be returned by readData. @@ -126,12 +126,12 @@ class CSndBuffer /// @param [in] kflags Odd|Even crypto key flag /// @param [out] seqnoinc the number of packets skipped due to TTL, so that seqno should be incremented. /// @return Actual length of data read. - SRT_ATTR_EXCLUDES(m_BufLock) + SRT_TSA_NEEDS_NONLOCKED(m_BufLock) int readData(CPacket& w_packet, time_point& w_origintime, int kflgs, int& w_seqnoinc); /// Peek an information on the next original data packet to send. /// @return origin time stamp of the next packet; epoch start time otherwise. - SRT_ATTR_EXCLUDES(m_BufLock) + SRT_TSA_NEEDS_NONLOCKED(m_BufLock) time_point peekNextOriginal() const; struct DropRange @@ -155,14 +155,14 @@ class CSndBuffer /// @retval >0 Length of the data read. /// @retval READ_NONE No data available or @a offset points out of the buffer occupied space. /// @retval READ_DROP The call requested data drop due to TTL exceeded, to be handled first. - SRT_ATTR_EXCLUDES(m_BufLock) + SRT_TSA_NEEDS_NONLOCKED(m_BufLock) int readData(const int offset, CPacket& w_packet, time_point& w_origintime, DropRange& w_drop); /// Get the time of the last retransmission (if any) of the DATA packet. /// @param [in] offset offset from the last ACK point (backward sequence number difference) /// /// @return Last time of the last retransmission event for the corresponding DATA packet. - SRT_ATTR_EXCLUDES(m_BufLock) + SRT_TSA_NEEDS_NONLOCKED(m_BufLock) time_point getPacketRexmitTime(const int offset); /// Update the ACK point and may release/unmap/return the user data according to the flag. @@ -175,7 +175,7 @@ class CSndBuffer /// @return Current size of the data in the sending list. int getCurrBufSize() const; - SRT_ATTR_EXCLUDES(m_BufLock) + SRT_TSA_NEEDS_NONLOCKED(m_BufLock) int dropLateData(int& bytes, int32_t& w_first_msgno, const time_point& too_late_time); void updAvgBufSize(const time_point& time); @@ -199,7 +199,7 @@ class CSndBuffer /// @brief Get the buffering delay of the oldest message in the buffer. /// @return the delay value. - SRT_ATTR_EXCLUDES(m_BufLock) + SRT_TSA_NEEDS_NONLOCKED(m_BufLock) duration getBufferingDelay(const time_point& tnow) const; uint64_t getInRatePeriod() const { return m_rateEstimator.getInRatePeriod(); } diff --git a/srtcore/core.cpp b/srtcore/core.cpp index eca2b2069..c92c0877e 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -259,6 +259,7 @@ CUDTUnited& srt::CUDT::uglobal() #endif +SRT_TSA_DISABLED void srt::CUDT::construct() { m_pSndBuffer = NULL; @@ -607,9 +608,8 @@ void srt::CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen) case SRTO_RCVDATA: if (m_pRcvBuffer) { - enterCS(m_RecvLock); + ScopedLock lck(m_RcvBufferLock); *(int32_t *)optval = m_pRcvBuffer->getRcvDataSize(); - leaveCS(m_RecvLock); } else *(int32_t *)optval = 0; @@ -954,6 +954,11 @@ void srt::CUDT::clearData() m_tsRcvPeerStartTime = steady_clock::time_point(); } +//This function is being called at the moment when no +// thread-related facilities for this socket are running. +// Hence thread checking is disabled. + +SRT_TSA_DISABLED void srt::CUDT::open() { ScopedLock cg(m_ConnectionLock); @@ -987,6 +992,7 @@ void srt::CUDT::open() m_tdNAKInterval = m_tdMinNakInterval; const steady_clock::time_point currtime = steady_clock::now(); + m_tsLastRspTime.store(currtime); m_tsNextACKTime.store(currtime + m_tdACKInterval); m_tsNextNAKTime.store(currtime + m_tdNAKInterval); @@ -1400,6 +1406,8 @@ size_t srt::CUDT::fillHsExtKMRSP(uint32_t* pcmdspec, const uint32_t* kmdata, siz keydata = failure_kmrsp; // Update the KM state as well + // NOTE: these fields are made atomic so that they can be safely written, + // but formally this should require lock on m_ConnectionLock. m_pCryptoControl->m_SndKmState = SRT_KM_S_NOSECRET; // Agent has PW, but Peer won't decrypt m_pCryptoControl->m_RcvKmState = SRT_KM_S_UNSECURED; // Peer won't encrypt as well. } @@ -1993,6 +2001,8 @@ RttTracer s_rtt_trace; bool srt::CUDT::processSrtMsg(const CPacket *ctrlpkt) { + // XXX ScopedLock clok (m_ConnectionLock); ??? + uint32_t *srtdata = (uint32_t *)ctrlpkt->m_pcData; size_t len = ctrlpkt->getLength(); int etype = ctrlpkt->getExtendedType(); @@ -2604,6 +2614,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, HLOGC(cnlog.Debug, log << CONID() << "interpretSrtHandshake: extracting KMREQ/RSP type extension"); #ifdef SRT_ENABLE_ENCRYPTION + // XXX ScopedLock clok (m_ConnectionLock); ??? if (!m_pCryptoControl->hasPassphrase()) { if (m_config.bEnforcedEnc) @@ -3275,6 +3286,10 @@ bool srt::CUDT::interpretGroup(const int32_t groupdata[], size_t data_size SRT_A // exclusively on the listener side (HSD_RESPONDER, HSv5+). // [[using locked(s_UDTUnited.m_GlobControlLock)]] +// XXX +// SRT_TSA_NEEDS_LOCKED(CUDTUnited::m_GlobControlLock) +// Can't be set because clang-tsa doesn't honor friend declarations +// Alternatively you can move it to CUDTSocket class. SRTSOCKET srt::CUDT::makeMePeerOf(SRTSOCKET peergroup, SRT_GROUP_TYPE gtp, uint32_t link_flags) { // Note: This function will lock pg->m_GroupLock! @@ -3308,7 +3323,7 @@ SRTSOCKET srt::CUDT::makeMePeerOf(SRTSOCKET peergroup, SRT_GROUP_TYPE gtp, uint3 { try { - gp = &newGroup(gtp); + gp = &uglobal().newGroup(gtp); } catch (...) { @@ -3425,6 +3440,9 @@ void srt::CUDT::synchronizeWithGroup(CUDTGroup* gp) // but between them there's a 30s distance, considered large enough // time to not fill a network window. enterCS(m_RecvLock); + // NOTE: Here is theoretically also the buffer lock required, but this + // function is called from acceptAndRespond when no modifications in the + // buffer or reading from any thread are for the time being possible. m_pRcvBuffer->applyGroupTime(rcv_buffer_time_base, rcv_buffer_wrap_period, m_iTsbPdDelay_ms * 1000, rcv_buffer_udrift); m_pRcvBuffer->setPeerRexmitFlag(m_bPeerRexmitFlag); leaveCS(m_RecvLock); @@ -4727,6 +4745,7 @@ bool srt::CUDT::applyResponseSettings(const CPacket* pHspkt /*[[nullable]]*/) AT m_iMaxSRTPayloadSize = m_config.iMSS - full_hdr_size; HLOGC(cnlog.Debug, log << CONID() << "applyResponseSettings: PAYLOAD SIZE: " << m_iMaxSRTPayloadSize); + // NOTE: m_RcvAckLock required, but this is here allowed because not all threads run yet m_iFlowWindowSize = m_ConnRes.m_iFlightFlagSize; const int udpsize = m_config.iMSS - CPacket::UDP_HDR_SIZE; m_iMaxSRTPayloadSize = udpsize - CPacket::HDR_SIZE; @@ -5450,6 +5469,7 @@ void * srt::CUDT::tsbpd(void* param) rxready = true; if (info.seq_gap) { + // XXX TSA: Requires lock on m_RcvBufferLock (locked already by enterCS) const int iDropCnt SRT_ATR_UNUSED = self->rcvDropTooLateUpTo(info.seqno); #if ENABLE_BONDING shall_update_group = true; @@ -5523,6 +5543,8 @@ void * srt::CUDT::tsbpd(void* param) // Functions called below will lock m_GroupLock, which in hierarchy // lies after m_RecvLock. Must unlock m_RecvLock to be able to lock // m_GroupLock inside the calls. + // XXX TSA will report this because it doesn't understand the + // annotation applied to a constructor or destructor. InvertedLock unrecv(self->m_RecvLock); // The current "APP reader" needs to simply decide as to whether // the next CUDTGroup::recv() call should return with no blocking or not. @@ -7493,7 +7515,9 @@ void srt::CUDT::bstats(CBytePerfMon *perf, bool clear, bool instantaneous) const int pktHdrSize = CPacket::HDR_SIZE + CPacket::UDP_HDR_SIZE; { + m_RecvAckLock.lock(); int32_t flight_span = getFlightSpan(); + m_RecvAckLock.unlock(); ScopedLock statsguard(m_StatsLock); @@ -7907,6 +7931,7 @@ void srt::CUDT::sendCtrl(UDTMessageType pkttype, const int32_t* lparam, void* rp case UMSG_LOSSREPORT: // 011 - Loss Report { + ScopedLock lock(m_RcvLossLock); // Explicitly defined lost sequences if (rparam) { @@ -7925,7 +7950,6 @@ void srt::CUDT::sendCtrl(UDTMessageType pkttype, const int32_t* lparam, void* rp // Call with no arguments - get loss list from internal data. else if (m_pRcvLossList->getLossLength() > 0) { - ScopedLock lock(m_RcvLossLock); // this is periodically NAK report; make sure NAK cannot be sent back too often // read loss list from the local receiver loss list @@ -9163,7 +9187,7 @@ void srt::CUDT::processCtrl(const CPacket &ctrlpkt) // Just heard from the peer, reset the expiration count. m_iEXPCount = 1; const steady_clock::time_point currtime = steady_clock::now(); - m_tsLastRspTime = currtime; + m_tsLastRspTime = currtime; // XXX Requires lock m_RecvAckLock? HLOGC(inlog.Debug, log << CONID() << "incoming UMSG:" << ctrlpkt.getType() << " (" @@ -9238,6 +9262,7 @@ void srt::CUDT::updateSrtRcvSettings() // (unlike in normal situation when reading directly from socket), however // its time to play shall be properly defined. ScopedLock lock(m_RecvLock); + // XXX ALSO: m_RcvBufferLock ??? // NOTE: remember to also update synchronizeWithGroup() if more settings are updated here. m_pRcvBuffer->setPeerRexmitFlag(m_bPeerRexmitFlag); @@ -9663,7 +9688,7 @@ bool srt::CUDT::packData(CPacket& w_packet, steady_clock::time_point& w_nexttime { IF_HEAVY_LOGGING(reason = "reXmit"); } - else if (m_PacketFilter && + else if (m_PacketFilter && // XXX m_iSndCurrSeqNo requires locking m_RcvAckLock m_PacketFilter.packControlPacket(m_iSndCurrSeqNo, m_pCryptoControl->getSndCryptoFlags(), (w_packet))) { HLOGC(qslog.Debug, log << CONID() << "filter: filter/CTL packet ready - packing instead of data."); @@ -10323,7 +10348,7 @@ int srt::CUDT::processData(CUnit* in_unit) // Just heard from the peer, reset the expiration count. m_iEXPCount = 1; - m_tsLastRspTime.store(steady_clock::now()); + m_tsLastRspTime.store(steady_clock::now()); // XXX Requires lock m_RecvAckLock // We are receiving data, start tsbpd thread if TsbPd is enabled @@ -10736,7 +10761,7 @@ void srt::CUDT::updateIdleLinkFrom(CUDT* source) { int bufseq; { - ScopedLock lg (m_RcvBufferLock); + ScopedLock lg (source->m_RcvBufferLock); bufseq = source->m_pRcvBuffer->getStartSeqNo(); } ScopedLock lg (m_RecvLock); @@ -11299,6 +11324,8 @@ int srt::CUDT::processConnectRequest(const sockaddr_any& addr, CPacket& packet) return conn; packet.setLength(m_iMaxSRTPayloadSize); + // XXX REQUIRES LOCK ON acpu->m_ConnectionLock. + // Check clashes with m_LSLock! if (!acpu->createSrtHandshake(SRT_CMD_HSRSP, SRT_CMD_KMRSP, kmdata, kmdatasize, (packet), (hs))) @@ -11488,6 +11515,8 @@ bool srt::CUDT::checkExpTimer(const steady_clock::time_point& currtime, int chec // In UDT the m_bUserDefinedRTO and m_iRTO were in CCC class. // There's nothing in the original code that alters these values. + // XXX lock on m_RecvAckLock ? + steady_clock::time_point next_exp_time; if (m_CongCtl->RTO()) { @@ -11604,27 +11633,34 @@ void srt::CUDT::checkRexmitTimer(const steady_clock::time_point& currtime) // (the receiver didn't send any LOSSREPORT, or LOSSREPORT was lost on track). // - in case of FASTREXMIT (Live Mode): the RTO (rtt_syn) was triggered, therefore // schedule unacknowledged packets for retransmission regardless of the loss list emptiness. - if (getFlightSpan() > 0 && (!is_laterexmit || m_pSndLossList->getLossLength() == 0)) + + if ((!is_laterexmit || m_pSndLossList->getLossLength() == 0)) { - // Sender: Insert all the packets sent after last received acknowledgement into the sender loss list. ScopedLock acklock(m_RecvAckLock); // Protect packet retransmission - // Resend all unacknowledged packets on timeout, but only if there is no packet in the loss list - const int32_t csn = m_iSndCurrSeqNo; - const int num = m_pSndLossList->insert(m_iSndLastAck, csn); - if (num > 0) + if (getFlightSpan() > 0) { - enterCS(m_StatsLock); - m_stats.sndr.lost.count(num); - leaveCS(m_StatsLock); + // Sender: Insert all the packets sent after last received acknowledgement into the sender loss list. + // Resend all unacknowledged packets on timeout, but only if there is no packet in the loss list + const int32_t csn = m_iSndCurrSeqNo; + const int num = m_pSndLossList->insert(m_iSndLastAck, csn); + if (num > 0) + { + enterCS(m_StatsLock); + m_stats.sndr.lost.count(num); + leaveCS(m_StatsLock); - HLOGC(xtlog.Debug, - log << CONID() << "ENFORCED " << (is_laterexmit ? "LATEREXMIT" : "FASTREXMIT") - << " by ACK-TMOUT (scheduling): " << CSeqNo::incseq(m_iSndLastAck) << "-" << csn << " (" - << CSeqNo::seqoff(m_iSndLastAck, csn) << " packets)"); + HLOGC(xtlog.Debug, + log << CONID() << "ENFORCED " << (is_laterexmit ? "LATEREXMIT" : "FASTREXMIT") + << " by ACK-TMOUT (scheduling): " << CSeqNo::incseq(m_iSndLastAck) << "-" << csn << " (" + << CSeqNo::seqoff(m_iSndLastAck, csn) << " packets)"); + } } } - ++m_iReXmitCount; + { + ScopedLock ack_lock(m_RecvAckLock); + ++m_iReXmitCount; + } const ECheckTimerStage stage = is_fastrexmit ? TEV_CHT_FASTREXMIT : TEV_CHT_REXMIT; updateCC(TEV_CHECKTIMER, EventVariant(stage)); diff --git a/srtcore/core.h b/srtcore/core.h index ed250c641..e86c85ddf 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -307,27 +307,29 @@ class CUDT SRTSOCKET socketID() const { return m_SocketID; } static CUDT* getUDTHandle(SRTSOCKET u); - static std::vector existingSockets(); + //static std::vector existingSockets(); XXX UNUSED. RESTORE IF NEEDED void addressAndSend(CPacket& pkt); - SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) void sendSrtMsg(int cmd, uint32_t *srtdata_in = NULL, size_t srtlen_in = 0); bool isOPT_TsbPd() const { return m_config.bTSBPD; } int SRTT() const { return m_iSRTT; } int RTTVar() const { return m_iRTTVar; } + SRT_TSA_NEEDS_LOCKED(m_RecvAckLock) int32_t sndSeqNo() const { return m_iSndCurrSeqNo; } int32_t schedSeqNo() const { return m_iSndNextSeqNo; } bool overrideSndSeqNo(int32_t seq); #if ENABLE_BONDING + SRT_TSA_NEEDS_LOCKED(m_RecvAckLock) sync::steady_clock::time_point lastRspTime() const { return m_tsLastRspTime.load(); } sync::steady_clock::time_point freshActivationStart() const { return m_tsFreshActivation; } #endif int32_t rcvSeqNo() const { return m_iRcvCurrSeqNo; } - SRT_ATTR_REQUIRES(m_RecvAckLock) + SRT_TSA_NEEDS_LOCKED(m_RecvAckLock) int flowWindowSize() const { return m_iFlowWindowSize; } int32_t deliveryRate() const { return m_iDeliveryRate; } int bandwidth() const { return m_iBandwidth; } @@ -389,7 +391,7 @@ class CUDT /// Returns the number of packets in flight (sent, but not yet acknowledged). /// @returns The number of packets in flight belonging to the interval [0; ...) - SRT_ATTR_REQUIRES(m_RecvAckLock) + SRT_TSA_NEEDS_LOCKED(m_RecvAckLock) int32_t getFlightSpan() const { return getFlightSpan(m_iSndLastAck, m_iSndCurrSeqNo); @@ -422,14 +424,14 @@ class CUDT /// @brief Set the timestamp field of the packet using the provided value (no check) /// @param p the packet structure to set the timestamp on. /// @param ts timestamp to use as a source for packet timestamp. - SRT_ATTR_EXCLUDES(m_StatsLock) + SRT_TSA_NEEDS_NONLOCKED(m_StatsLock) void setPacketTS(CPacket& p, const time_point& ts); /// @brief Set the timestamp field of the packet according the TSBPD mode. /// Also checks the connection start time (m_tsStartTime). /// @param p the packet structure to set the timestamp on. /// @param ts timestamp to use as a source for packet timestamp. Ignored if m_bPeerTsbPd is false. - SRT_ATTR_EXCLUDES(m_StatsLock) + SRT_TSA_NEEDS_NONLOCKED(m_StatsLock) void setDataPacketTS(CPacket& p, const time_point& ts); // Utility used for closing a listening socket @@ -493,7 +495,7 @@ class CUDT /// @retval 0 Connection successful /// @retval 1 Connection in progress (m_ConnReq turned into RESPONSE) /// @retval -1 Connection failed - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) EConnectStatus processConnectResponse(const CPacket& pkt, CUDTException* eout) ATR_NOEXCEPT; // This function works in case of HSv5 rendezvous. It changes the state @@ -513,20 +515,21 @@ class CUDT /// @param response incoming handshake response packet to be interpreted /// @param serv_addr incoming packet's address /// @param rst Current read status to know if the HS packet was freshly received from the peer, or this is only a periodic update (RST_AGAIN) - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) EConnectStatus processRendezvous(const CPacket* response, const sockaddr_any& serv_addr, EReadStatus, CPacket& reqpkt); void sendRendezvousRejection(const sockaddr_any& serv_addr, CPacket& request); /// Create the CryptoControl object based on the HS packet. - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) bool prepareConnectionObjects(const CHandShake &hs, HandshakeSide hsd, CUDTException* eout); /// Allocates sender and receiver buffers and loss lists. - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) bool prepareBuffers(CUDTException* eout); + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) int getAuthTagSize() const; - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) EConnectStatus postConnect(const CPacket* response, bool rendezvous, CUDTException* eout) ATR_NOEXCEPT; SRT_ATR_NODISCARD bool applyResponseSettings(const CPacket* hspkt /*[[nullable]]*/) ATR_NOEXCEPT; @@ -540,17 +543,20 @@ class CUDT SRT_ATR_NODISCARD size_t fillSrtHandshake_HSRSP(uint32_t* srtdata, size_t srtlen, int hs_version); SRT_ATR_NODISCARD size_t fillSrtHandshake(uint32_t* srtdata, size_t srtlen, int msgtype, int hs_version); - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) bool createSrtHandshake(int srths_cmd, int srtkm_cmd, const uint32_t* data, size_t datalen, CPacket& w_reqpkt, CHandShake& w_hs); + SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) SRT_ATR_NODISCARD size_t fillHsExtConfigString(uint32_t *pcmdspec, int cmd, const std::string &str); #if ENABLE_BONDING + SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) SRT_ATR_NODISCARD size_t fillHsExtGroup(uint32_t *pcmdspec); #endif - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) size_t fillHsExtKMREQ(uint32_t *pcmdspec, size_t ki); + SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) SRT_ATR_NODISCARD size_t fillHsExtKMRSP(uint32_t *pcmdspec, const uint32_t *kmdata, size_t kmdata_wordsize); SRT_ATR_NODISCARD size_t prepareSrtHsMsg(int cmd, uint32_t* srtdata, size_t size); @@ -562,11 +568,14 @@ class CUDT SRT_ATR_NODISCARD bool checkApplyFilterConfig(const std::string& cs); #if ENABLE_BONDING - static CUDTGroup& newGroup(const int); // defined EXCEPTIONALLY in api.cpp for convenience reasons // Note: This is an "interpret" function, which should treat the tp as // "possibly group type" that might be out of the existing values. SRT_ATR_NODISCARD bool interpretGroup(const int32_t grpdata[], size_t data_size, int hsreq_type_cmd); - SRT_ATR_NODISCARD SRTSOCKET makeMePeerOf(SRTSOCKET peergroup, SRT_GROUP_TYPE tp, uint32_t link_flags); + SRT_ATR_NODISCARD + // XXX These below can't be set because the header file has no access to these field definitions + //SRT_TSA_NEEDS_LOCKED(CUDTUnited::m_GlobControlLock) + //SRT_TSA_NEEDS_NONLOCKED(CUDTGroup::m_GroupLock) + SRTSOCKET makeMePeerOf(SRTSOCKET peergroup, SRT_GROUP_TYPE tp, uint32_t link_flags); void synchronizeWithGroup(CUDTGroup* grp); #endif @@ -579,7 +588,9 @@ class CUDT /// @brief Drop packets too late to be delivered if any. /// @returns the number of packets actually dropped. - SRT_ATTR_REQUIRES2(m_RecvAckLock, m_StatsLock) + SRT_TSA_NEEDS_NONLOCKED(m_RecvAckLock) + SRT_TSA_NEEDS_NONLOCKED(m_StatsLock) + SRT_TSA_NEEDS_LOCKED(m_SendLock) int sndDropTooLate(); /// @bried Allow packet retransmission. @@ -698,12 +709,15 @@ class CUDT /// removes the loss record from both current receiver loss list and /// the receiver fresh loss list. void unlose(const CPacket& oldpacket); + + SRT_TSA_NEEDS_NONLOCKED(m_RcvLossLock) // will scope-lock it inside void dropFromLossLists(int32_t from, int32_t to); - SRT_ATTR_REQUIRES(m_RecvAckLock) + SRT_TSA_NEEDS_LOCKED(m_RcvBufferLock) + SRT_TSA_NEEDS_NONLOCKED(m_RcvLossLock) bool getFirstNoncontSequence(int32_t& w_seq, std::string& w_log_reason); - SRT_ATTR_EXCLUDES(m_ConnectionLock) + SRT_TSA_NEEDS_NONLOCKED(m_ConnectionLock) void checkSndTimers(); /// @brief Check and perform KM refresh if needed. @@ -750,13 +764,13 @@ class CUDT return m_stats.tsStartTime; } - SRT_ATTR_EXCLUDES(m_RcvBufferLock) + SRT_TSA_NEEDS_NONLOCKED(m_RcvBufferLock) bool isRcvBufferReady() const; - SRT_ATTR_REQUIRES(m_RcvBufferLock) + SRT_TSA_NEEDS_LOCKED(m_RcvBufferLock) bool isRcvBufferReadyNoLock() const; - SRT_ATTR_EXCLUDES(m_RcvBufferLock) + SRT_TSA_NEEDS_NONLOCKED(m_RcvBufferLock) bool isRcvBufferFull() const; // TSBPD thread main function. @@ -773,7 +787,8 @@ class CUDT /// @param seqno [in] The sequence number of the first packets following those to be dropped. /// @param reason A reason for dropping (see @a DropReason). /// @return The number of packets dropped. - SRT_ATTR_EXCLUDES(m_RcvBufferLock, m_RcvLossLock) + SRT_TSA_NEEDS_LOCKED(m_RcvBufferLock) + SRT_TSA_NEEDS_NONLOCKED(m_RcvLossLock) int rcvDropTooLateUpTo(int seqno, DropReason reason = DROP_TOO_LATE); static loss_seqs_t defaultPacketArrival(void* vself, CPacket& pkt); @@ -807,7 +822,12 @@ class CUDT int m_iTsbPdDelay_ms; // Rx delay to absorb burst, in milliseconds int m_iPeerTsbPdDelay_ms; // Tx delay that the peer uses to absorb burst, in milliseconds bool m_bTLPktDrop; // Enable Too-late Packet Drop - SRT_ATTR_PT_GUARDED_BY(m_ConnectionLock) + + // XXX Controversial because it is being stated + // that it should be guarded during creation in the + // initial time, but it's not guarded during dispatching + // of the handshake. + SRT_TSA_PT_GUARDED_BY(m_ConnectionLock) UniquePtr m_pCryptoControl; // Crypto control module CCache* m_pCache; // Network information cache @@ -867,7 +887,7 @@ class CUDT atomic_duration m_tdSendTimeDiff; // Aggregate difference in inter-packet sending time - SRT_ATTR_GUARDED_BY(m_RecvAckLock) + SRT_TSA_GUARDED_BY(m_RecvAckLock) sync::atomic m_iFlowWindowSize; // Flow control window size sync::atomic m_iCongestionWindow; // Congestion window size @@ -878,7 +898,9 @@ class CUDT duration m_tdACKInterval; // ACK interval duration m_tdNAKInterval; // NAK interval - SRT_ATTR_GUARDED_BY(m_RecvAckLock) + // XXX Controversial because it is often updated without locking, + // but then it holds the time when the response has come. + SRT_TSA_GUARDED_BY(m_RecvAckLock) atomic_time_point m_tsLastRspTime; // Timestamp of last response from the peer time_point m_tsLastRspAckTime; // (SND) Timestamp of last ACK from the peer atomic_time_point m_tsLastSndTime; // Timestamp of last data/ctrl sent (in system ticks) @@ -896,7 +918,7 @@ class CUDT time_point m_tsNextSendTime; // Scheduled time of next packet sending sync::atomic m_iSndLastFullAck; // Last full ACK received - SRT_ATTR_GUARDED_BY(m_RecvAckLock) + SRT_TSA_GUARDED_BY(m_RecvAckLock) sync::atomic m_iSndLastAck; // Last ACK received // NOTE: m_iSndLastDataAck is the value strictly bound to the CSndBufer object (m_pSndBuffer) @@ -908,7 +930,7 @@ class CUDT // require only the lost sequence number, and how to find the packet with this sequence // will be up to the sending buffer. sync::atomic m_iSndLastDataAck; // The real last ACK that updates the sender buffer and loss list - SRT_ATTR_GUARDED_BY(m_RecvAckLock) + SRT_TSA_GUARDED_BY(m_RecvAckLock) sync::atomic m_iSndCurrSeqNo; // The largest sequence number that HAS BEEN SENT sync::atomic m_iSndNextSeqNo; // The sequence number predicted to be placed at the currently scheduled packet @@ -926,6 +948,8 @@ class CUDT int32_t m_iSndLastAck2; // Last ACK2 sent back time_point m_SndLastAck2Time; // The time when last ACK2 was sent back + + SRT_TSA_DISABLED // becaue this should be run only on initialization void setInitialSndSeq(int32_t isn) { m_iSndLastAck = isn; @@ -944,7 +968,7 @@ class CUDT bool m_bPeerNakReport; // Sender's peer (receiver) issues Periodic NAK Reports bool m_bPeerRexmitFlag; // Receiver supports rexmit flag in payload packets - SRT_ATTR_GUARDED_BY(m_RecvAckLock) + SRT_TSA_GUARDED_BY(m_RecvAckLock) int32_t m_iReXmitCount; // Re-Transmit Count since last ACK static const size_t @@ -965,11 +989,11 @@ class CUDT bool frequentLogAllowed(size_t logid, const time_point& tnow, std::string& why); private: // Receiving related data - SRT_ATTR_GUARDED_BY(m_RcvBufferLock) + SRT_TSA_PT_GUARDED_BY(m_RcvBufferLock) CRcvBuffer* m_pRcvBuffer; //< Receiver buffer - SRT_ATTR_GUARDED_BY(m_RcvLossLock) + SRT_TSA_PT_GUARDED_BY(m_RcvLossLock) CRcvLossList* m_pRcvLossList; //< Receiver loss list - SRT_ATTR_GUARDED_BY(m_RcvLossLock) + SRT_TSA_GUARDED_BY(m_RcvLossLock) std::deque m_FreshLoss; //< Lost sequence already added to m_pRcvLossList, but not yet sent UMSG_LOSSREPORT for. int m_iReorderTolerance; //< Current value of dynamic reorder tolerance @@ -996,7 +1020,7 @@ class CUDT bool m_bTsbPd; // Peer sends TimeStamp-Based Packet Delivery Packets bool m_bGroupTsbPd; // TSBPD should be used for GROUP RECEIVER instead - SRT_ATTR_GUARDED_BY(m_RcvTsbPdStartupLock) + SRT_TSA_GUARDED_BY(m_RcvTsbPdStartupLock) sync::CThread m_RcvTsbPdThread; // Rcv TsbPD Thread handle sync::Condition m_RcvTsbPdCond; // TSBPD signals if reading is ready. Use together with m_RecvLock bool m_bTsbPdNeedsWakeup; // Signal TsbPd thread to wake up on RCV buffer state change. @@ -1060,7 +1084,7 @@ class CUDT // Failure to create the crypter means that an encrypted // connection should be rejected if ENFORCEDENCRYPTION is on. - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + SRT_ATR_NODISCARD SRT_TSA_NEEDS_LOCKED(m_ConnectionLock) bool createCrypter(HandshakeSide side, bool bidi); private: // Generation and processing of packets @@ -1132,7 +1156,7 @@ class CUDT bool packData(CPacket& packet, time_point& nexttime, sockaddr_any& src_addr); /// Also excludes srt::CUDTUnited::m_GlobControlLock. - SRT_ATTR_EXCLUDES(m_RcvTsbPdStartupLock, m_StatsLock, m_RecvLock, m_RcvLossLock, m_RcvBufferLock) + SRT_TSA_NEEDS_NONLOCKED(m_RcvTsbPdStartupLock, m_StatsLock, m_RecvLock, m_RcvLossLock, m_RcvBufferLock) int processData(CUnit* unit); /// This function passes the incoming packet to the initial processing @@ -1148,6 +1172,7 @@ class CUDT /// @return 0 The call was successful (regardless if the packet was accepted or not). /// @return -1 The call has failed: no space left in the buffer. /// @return -2 The incoming packet exceeds the expected sequence by more than a length of the buffer (irrepairable discrepancy). + SRT_TSA_NEEDS_LOCKED(m_RcvBufferLock) int handleSocketPacketReception(const std::vector& incoming, bool& w_new_inserted, bool& w_was_sent_in_order, CUDT::loss_seqs_t& w_srt_loss_seqs); /// Get the packet's TSBPD time - @@ -1156,7 +1181,7 @@ class CUDT /// and shall not be used when ENABLE_BONDING=0. time_point getPktTsbPdTime(void* grp, const CPacket& packet); - SRT_ATTR_EXCLUDES(m_RcvTsbPdStartupLock) + SRT_TSA_NEEDS_NONLOCKED(m_RcvTsbPdStartupLock) /// Checks and spawns the TSBPD thread if required. int checkLazySpawnTsbPdThread(); @@ -1176,9 +1201,9 @@ class CUDT void processKeepalive(const CPacket& ctrlpkt, const time_point& tsArrival); - SRT_ATTR_REQUIRES(m_RcvBufferLock) /// Retrieves the available size of the receiver buffer. /// Expects that m_RcvBufferLock is locked. + SRT_TSA_NEEDS_LOCKED(m_RcvBufferLock) size_t getAvailRcvBufferSizeNoLock() const; private: // Trace diff --git a/srtcore/crypto.h b/srtcore/crypto.h index 613ded8dd..4fca16eb3 100644 --- a/srtcore/crypto.h +++ b/srtcore/crypto.h @@ -59,8 +59,8 @@ class CCryptoControl // Temporarily allow these to be accessed. public: - SRT_KM_STATE m_SndKmState; //Sender Km State (imposed by agent) - SRT_KM_STATE m_RcvKmState; //Receiver Km State (informed by peer) + sync::atomic m_SndKmState; //Sender Km State (imposed by agent) + sync::atomic m_RcvKmState; //Receiver Km State (informed by peer) private: // Partial haicrypt configuration, consider @@ -120,7 +120,7 @@ class CCryptoControl /// Regenerate cryptographic key material if needed. /// @param[in] sock If not null, the socket will be used to send the KM message to the peer (e.g. KM refresh). /// @param[in] bidirectional If true, the key material will be regenerated for both directions (receiver and sender). - SRT_ATTR_EXCLUDES(m_mtxLock) + SRT_TSA_NEEDS_NONLOCKED(m_mtxLock) void regenCryptoKm(CUDT* sock, bool bidirectional); size_t KeyLen() { return m_iSndKmKeyLen; } @@ -212,7 +212,7 @@ class CCryptoControl std::string FormatKmMessage(std::string hdr, int cmd, size_t srtlen); bool init(HandshakeSide, const CSrtConfig&, bool bidir, bool bUseGcm153); - SRT_ATTR_EXCLUDES(m_mtxLock) + SRT_TSA_NEEDS_NONLOCKED(m_mtxLock) void close(); /// (Re)send KM request to a peer on timeout. @@ -221,7 +221,7 @@ class CCryptoControl /// - The case of key regeneration (KM refresh), when a new key has to be sent again. /// In this case the first sending happens in regenCryptoKm(..). This function /// retransmits the KM request by timeout if not KM response has been received. - SRT_ATTR_EXCLUDES(m_mtxLock) + SRT_TSA_NEEDS_NONLOCKED(m_mtxLock) void sendKeysToPeer(CUDT* sock, int iSRTT); void setCryptoSecret(const HaiCrypt_Secret& secret) diff --git a/srtcore/group.cpp b/srtcore/group.cpp index 5601cdeee..dc0d33b52 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -1065,8 +1065,11 @@ void CUDTGroup::close() // CSync::lock_notify_one(m_RcvDataCond, m_RcvDataLock); } -// [[using locked(m_Global->m_GlobControlLock)]] +// [[using locked(m_Global.m_GlobControlLock)]] // [[using locked(m_GroupLock)]] +// XXX TSA blocked because it causes errors on some versions of clang +//SRT_TSA_NEEDS_LOCKED(CUDTGroup::m_Global.m_GlobControlLock) +//SRT_TSA_NEEDS_LOCKED(CUDTGroup::m_GroupLock) void CUDTGroup::send_CheckValidSockets() { vector toremove; @@ -1940,6 +1943,7 @@ struct FLookupSocketWithEvent_LOCKED typedef CUDTSocket* result_type; + SRT_TSA_NEEDS_LOCKED(glob->m_GlobControlLock) pair operator()(const pair& es) { CUDTSocket* so = NULL; diff --git a/srtcore/logging.h b/srtcore/logging.h index 3f4efb286..c8d120e66 100644 --- a/srtcore/logging.h +++ b/srtcore/logging.h @@ -131,10 +131,10 @@ struct LogConfig { } - SRT_ATTR_ACQUIRE(mutex) + SRT_TSA_WILL_LOCK(mutex) void lock() const { mutex.lock(); } - SRT_ATTR_RELEASE(mutex) + SRT_TSA_WILL_UNLOCK(mutex) void unlock() const { mutex.unlock(); } }; diff --git a/srtcore/srt_attr_defs.h b/srtcore/srt_attr_defs.h index 726c4a03b..b67c05246 100644 --- a/srtcore/srt_attr_defs.h +++ b/srtcore/srt_attr_defs.h @@ -31,15 +31,8 @@ used by SRT library internally. #define ATR_DEPRECATED #endif -#if HAVE_CXX11 -#define SRT_ATR_ALIGNAS(n) alignas(n) -#elif HAVE_GCC -#define SRT_ATR_ALIGNAS(n) __attribute__((aligned(n))) -#else -#define SRT_ATR_ALIGNAS(n) -#endif - -#if defined(__cplusplus) && __cplusplus > 199711L +#if (defined(__cplusplus) && __cplusplus > 199711L) \ + || (defined(_MSVC_LANG) && _MSVC_LANG > 199711L) // Some earlier versions get this wrong #define HAVE_CXX11 1 // For gcc 4.7, claim C++11 is supported, as long as experimental C++0x is on, // however it's only the "most required C++11 support". @@ -86,6 +79,15 @@ used by SRT library internally. #define ATR_FINAL #endif // __cplusplus + +#if HAVE_CXX11 +#define SRT_ATR_ALIGNAS(n) alignas(n) +#elif HAVE_GCC +#define SRT_ATR_ALIGNAS(n) __attribute__((aligned(n))) +#else +#define SRT_ATR_ALIGNAS(n) +#endif + #if !HAVE_CXX11 && defined(REQUIRE_CXX11) && REQUIRE_CXX11 == 1 #error "The currently compiled application required C++11, but your compiler doesn't support it." #endif @@ -96,105 +98,126 @@ used by SRT library internally. // - MSVC SAL (partially). // - Other compilers: none. /////////////////////////////////////////////////////////////////////////////// + +// TSA SYMBOLS available: +// +// * SRT_TSA_CAPABILITY(x) +// The defined C++ class type has a lockable object capability. +// +// * SRT_TSA_SCOPED_CAPABILITY +// The defined C++ class type has a scoped locking capability. +// +// * SRT_TSA_GUARDED_BY(x) +// Accessing THIS object requires locking x for access. +// +// * SRT_TSA_PT_GUARDED_BY(x) +// The pointer-type field points to an object that should be guarded access by x +// +// * SRT_TSA_LOCK_ORDERS_BEFORE(...) +// THIS mutex must be locked prior to locking given mutex objects +// +// * SRT_TSA_LOCK_ORDERS_AFTER(...) +// THIS mutex must be locked next to locking given mutex objects +// +// * SRT_TSA_NEEDS_LOCKED(...) +// This function requires that given mutexes be locked prior to calling it +// +// * SRT_TSA_NEEDS_LOCKED2(...) +// Same as SRT_TSA_NEEDS_LOCKED, provided for portability with MSVC +// +// * SRT_TSA_NEEDS_LOCKED_SHARED(...) +// Same as SRT_TSA_NEEDS_LOCKED, but requires a shared lock. +// +// * SRT_TSA_WILL_LOCK(...) +// Declares that after this function has been called, it will leave given mutexes locked. +// +// * SRT_TSA_WILL_LOCK_SHARED(...) +// Like SRT_TSA_WILL_LOCK, but applies to a shared lock +// +// * SRT_TSA_WILL_UNLOCK(...) +// Declares that this function's call will leave given mutexes unlocked. +// +// * SRT_TSA_WILL_UNLOCK_SHARED(...) +// Like SRT_TSA_WILL_UNLOCK, but a shared lock. +// +// * SRT_TSA_WILL_UNLOCK_GENERIC(...) +// Like SRT_TSA_WILL_UNLOCK, but any kind of lock. +// +// * SRT_TSA_WILL_TRY_LOCK(...) +// * SRT_TSA_WILL_TRY_LOCK_SHARED(...) +// This function will try to lock and leave with locked if succeeded +// +// * SRT_TSA_NEEDS_NONLOCKED(...) +// Requires that to call this function the given mutexes must not be locked. +// +// * SRT_TSA_ASSERT_CAPABILITY(x) +// * SRT_TSA_ASSERT_SHARED_CAPABILITY(x) +// Will assert that the mutex is locked + +// * SRT_TSA_RETURN_CAPABILITY(x) +// This function will return an access to an object that is a mutex. + +// * SRT_TSA_DISABLED +// For this function the TSA will not be done. + #if _MSC_VER >= 1920 // In case of MSVC these attributes have to precede the attributed objects (variable, function). -// E.g. SRT_ATTR_GUARDED_BY(mtx) int object; +// E.g. SRT_TSA_GUARDED_BY(mtx) int object; // It is tricky to annotate e.g. the following function, as clang complaints it does not know 'm'. -// SRT_ATTR_EXCLUDES(m) SRT_ATTR_ACQUIRE(m) +// SRT_TSA_NEEDS_NONLOCKED(m) SRT_TSA_WILL_LOCK(m) // inline void enterCS(Mutex& m) { m.lock(); } -#define SRT_ATTR_CAPABILITY(expr) -#define SRT_ATTR_SCOPED_CAPABILITY -#define SRT_ATTR_GUARDED_BY(expr) _Guarded_by_(expr) -#define SRT_ATTR_PT_GUARDED_BY(expr) -#define SRT_ATTR_ACQUIRED_BEFORE(...) -#define SRT_ATTR_ACQUIRED_AFTER(...) -#define SRT_ATTR_REQUIRES(expr) _Requires_lock_held_(expr) -#define SRT_ATTR_REQUIRES2(expr1, expr2) _Requires_lock_held_(expr1) _Requires_lock_held_(expr2) -#define SRT_ATTR_REQUIRES_SHARED(...) -#define SRT_ATTR_ACQUIRE(expr) _Acquires_nonreentrant_lock_(expr) -#define SRT_ATTR_ACQUIRE_SHARED(...) -#define SRT_ATTR_RELEASE(expr) _Releases_lock_(expr) -#define SRT_ATTR_RELEASE_SHARED(...) -#define SRT_ATTR_RELEASE_GENERIC(...) -#define SRT_ATTR_TRY_ACQUIRE(...) _Acquires_nonreentrant_lock_(expr) -#define SRT_ATTR_TRY_ACQUIRE_SHARED(...) -#define SRT_ATTR_EXCLUDES(...) // the caller must not hold the given capabilities -#define SRT_ATTR_ASSERT_CAPABILITY(expr) -#define SRT_ATTR_ASSERT_SHARED_CAPABILITY(x) -#define SRT_ATTR_RETURN_CAPABILITY(x) -#define SRT_ATTR_NO_THREAD_SAFETY_ANALYSIS +#define SRT_TSA_CAPABILITY(expr) +#define SRT_TSA_SCOPED_CAPABILITY +#define SRT_TSA_GUARDED_BY(expr) _Guarded_by_(expr) +#define SRT_TSA_PT_GUARDED_BY(expr) +#define SRT_TSA_LOCK_ORDERS_BEFORE(...) +#define SRT_TSA_LOCK_ORDERS_AFTER(...) +#define SRT_TSA_NEEDS_LOCKED(expr) _Requires_lock_held_(expr) +#define SRT_TSA_NEEDS_LOCKED2(expr1, expr2) _Requires_lock_held_(expr1) _Requires_lock_held_(expr2) +#define SRT_TSA_NEEDS_LOCKED_SHARED(...) +#define SRT_TSA_WILL_LOCK(expr) _Acquires_nonreentrant_lock_(expr) +#define SRT_TSA_WILL_LOCK_SHARED(...) +#define SRT_TSA_WILL_UNLOCK(expr) _Releases_lock_(expr) +#define SRT_TSA_WILL_UNLOCK_SHARED(...) +#define SRT_TSA_WILL_UNLOCK_GENERIC(...) +#define SRT_TSA_WILL_TRY_LOCK(...) _Acquires_nonreentrant_lock_(expr) +#define SRT_TSA_WILL_TRY_LOCK_SHARED(...) +#define SRT_TSA_NEEDS_NONLOCKED(...) +#define SRT_TSA_ASSERT_CAPABILITY(expr) +#define SRT_TSA_ASSERT_SHARED_CAPABILITY(x) +#define SRT_TSA_RETURN_CAPABILITY(x) +#define SRT_TSA_DISABLED #else +// Common for clang supporting TCA and unsupported. #if defined(__clang__) && defined(__clang_major__) && (__clang_major__ > 5) -#define THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x)) +#define SRT_TSA_EXPR(x) __attribute__((x)) #else -#define THREAD_ANNOTATION_ATTRIBUTE__(x) // no-op +#define SRT_TSA_EXPR(x) // no-op #endif -#define SRT_ATTR_CAPABILITY(x) \ - THREAD_ANNOTATION_ATTRIBUTE__(capability(x)) - -#define SRT_ATTR_SCOPED_CAPABILITY \ - THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable) - -#define SRT_ATTR_GUARDED_BY(x) \ - THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x)) - -#define SRT_ATTR_PT_GUARDED_BY(x) \ - THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x)) - -#define SRT_ATTR_ACQUIRED_BEFORE(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(__VA_ARGS__)) - -#define SRT_ATTR_ACQUIRED_AFTER(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(acquired_after(__VA_ARGS__)) - -#define SRT_ATTR_REQUIRES(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(requires_capability(__VA_ARGS__)) - -#define SRT_ATTR_REQUIRES2(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(requires_capability(__VA_ARGS__)) - -#define SRT_ATTR_REQUIRES_SHARED(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(requires_shared_capability(__VA_ARGS__)) - -#define SRT_ATTR_ACQUIRE(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(acquire_capability(__VA_ARGS__)) - -#define SRT_ATTR_ACQUIRE_SHARED(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(acquire_shared_capability(__VA_ARGS__)) - -#define SRT_ATTR_RELEASE(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(release_capability(__VA_ARGS__)) - -#define SRT_ATTR_RELEASE_SHARED(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(release_shared_capability(__VA_ARGS__)) - -#define SRT_ATTR_RELEASE_GENERIC(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(release_generic_capability(__VA_ARGS__)) - -#define SRT_ATTR_TRY_ACQUIRE(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_capability(__VA_ARGS__)) - -#define SRT_ATTR_TRY_ACQUIRE_SHARED(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_shared_capability(__VA_ARGS__)) - +#define SRT_TSA_CAPABILITY(x) SRT_TSA_EXPR(capability(x)) +#define SRT_TSA_SCOPED_CAPABILITY SRT_TSA_EXPR(scoped_lockable) +#define SRT_TSA_GUARDED_BY(x) SRT_TSA_EXPR(guarded_by(x)) +#define SRT_TSA_PT_GUARDED_BY(x) SRT_TSA_EXPR(pt_guarded_by(x)) +#define SRT_TSA_LOCK_ORDERS_BEFORE(...) SRT_TSA_EXPR(acquired_before(__VA_ARGS__)) +#define SRT_TSA_LOCK_ORDERS_AFTER(...) SRT_TSA_EXPR(acquired_after(__VA_ARGS__)) +#define SRT_TSA_NEEDS_LOCKED(...) SRT_TSA_EXPR(requires_capability(__VA_ARGS__)) +#define SRT_TSA_NEEDS_LOCKED2(...) SRT_TSA_EXPR(requires_capability(__VA_ARGS__)) +#define SRT_TSA_NEEDS_LOCKED_SHARED(...) SRT_TSA_EXPR(requires_shared_capability(__VA_ARGS__)) +#define SRT_TSA_WILL_LOCK(...) SRT_TSA_EXPR(acquire_capability(__VA_ARGS__)) +#define SRT_TSA_WILL_LOCK_SHARED(...) SRT_TSA_EXPR(acquire_shared_capability(__VA_ARGS__)) +#define SRT_TSA_WILL_UNLOCK(...) SRT_TSA_EXPR(release_capability(__VA_ARGS__)) +#define SRT_TSA_WILL_UNLOCK_SHARED(...) SRT_TSA_EXPR(release_shared_capability(__VA_ARGS__)) +#define SRT_TSA_WILL_UNLOCK_GENERIC(...) SRT_TSA_EXPR(release_generic_capability(__VA_ARGS__)) +#define SRT_TSA_WILL_TRY_LOCK(...) SRT_TSA_EXPR(try_acquire_capability(__VA_ARGS__)) +#define SRT_TSA_WILL_TRY_LOCK_SHARED(...) SRT_TSA_EXPR(try_acquire_shared_capability(__VA_ARGS__)) +#define SRT_TSA_NEEDS_NONLOCKED(...) SRT_TSA_EXPR(locks_excluded(__VA_ARGS__)) +#define SRT_TSA_ASSERT_CAPABILITY(x) SRT_TSA_EXPR(assert_capability(x)) +#define SRT_TSA_ASSERT_SHARED_CAPABILITY(x) SRT_TSA_EXPR(assert_shared_capability(x)) +#define SRT_TSA_RETURN_CAPABILITY(x) SRT_TSA_EXPR(lock_returned(x)) +#define SRT_TSA_DISABLED SRT_TSA_EXPR(no_thread_safety_analysis) // The caller must not hold the given capabilities. -#define SRT_ATTR_EXCLUDES(...) \ - THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__)) - -#define SRT_ATTR_ASSERT_CAPABILITY(x) \ - THREAD_ANNOTATION_ATTRIBUTE__(assert_capability(x)) - -#define SRT_ATTR_ASSERT_SHARED_CAPABILITY(x) \ - THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_capability(x)) - -#define SRT_ATTR_RETURN_CAPABILITY(x) \ - THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x)) - -#define SRT_ATTR_NO_THREAD_SAFETY_ANALYSIS \ - THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis) - #endif // not _MSC_VER #endif // INC_SRT_ATTR_DEFS_H diff --git a/srtcore/sync.h b/srtcore/sync.h index 9d282304c..8f08e3914 100644 --- a/srtcore/sync.h +++ b/srtcore/sync.h @@ -321,7 +321,7 @@ using ScopedLock = std::lock_guard; /// Mutex is a class wrapper, that should mimic the std::chrono::mutex class. /// At the moment the extra function ref() is temporally added to allow calls /// to pthread_cond_timedwait(). Will be removed by introducing CEvent. -class SRT_ATTR_CAPABILITY("mutex") Mutex +class SRT_TSA_CAPABILITY("mutex") Mutex { friend class SyncEvent; @@ -330,11 +330,11 @@ class SRT_ATTR_CAPABILITY("mutex") Mutex ~Mutex(); public: - int lock() SRT_ATTR_ACQUIRE(); - int unlock() SRT_ATTR_RELEASE(); + int lock() SRT_TSA_WILL_LOCK(); + int unlock() SRT_TSA_WILL_UNLOCK(); /// @return true if the lock was acquired successfully, otherwise false - bool try_lock() SRT_ATTR_TRY_ACQUIRE(true); + bool try_lock() SRT_TSA_WILL_TRY_LOCK(true); // TODO: To be removed with introduction of the CEvent. pthread_mutex_t& ref() { return m_mutex; } @@ -344,17 +344,17 @@ class SRT_ATTR_CAPABILITY("mutex") Mutex }; /// A pthread version of std::scoped_lock (or lock_guard for C++11). -class SRT_ATTR_SCOPED_CAPABILITY ScopedLock +class SRT_TSA_SCOPED_CAPABILITY ScopedLock { public: - SRT_ATTR_ACQUIRE(m) + SRT_TSA_WILL_LOCK(m) explicit ScopedLock(Mutex& m) : m_mutex(m) { m_mutex.lock(); } - SRT_ATTR_RELEASE() + SRT_TSA_WILL_UNLOCK() ~ScopedLock() { m_mutex.unlock(); } private: @@ -362,50 +362,60 @@ class SRT_ATTR_SCOPED_CAPABILITY ScopedLock }; /// A pthread version of std::chrono::unique_lock -class SRT_ATTR_SCOPED_CAPABILITY UniqueLock +class SRT_TSA_SCOPED_CAPABILITY UniqueLock { friend class SyncEvent; int m_iLocked; Mutex& m_Mutex; public: - SRT_ATTR_ACQUIRE(m) + SRT_TSA_WILL_LOCK(m) explicit UniqueLock(Mutex &m); - SRT_ATTR_RELEASE() + SRT_TSA_WILL_UNLOCK() ~UniqueLock(); public: - SRT_ATTR_ACQUIRE() + SRT_TSA_WILL_LOCK() void lock(); - SRT_ATTR_RELEASE() + SRT_TSA_WILL_UNLOCK() void unlock(); - SRT_ATTR_RETURN_CAPABILITY(m_Mutex) + SRT_TSA_RETURN_CAPABILITY(m_Mutex) Mutex* mutex(); // reflects C++11 unique_lock::mutex() }; #endif // ENABLE_STDCXX_SYNC -inline void enterCS(Mutex& m) SRT_ATTR_EXCLUDES(m) SRT_ATTR_ACQUIRE(m) { m.lock(); } +inline void enterCS(Mutex& m) +SRT_TSA_NEEDS_NONLOCKED(m) +SRT_TSA_WILL_LOCK(m) +{ m.lock(); } -inline bool tryEnterCS(Mutex& m) SRT_ATTR_EXCLUDES(m) SRT_ATTR_TRY_ACQUIRE(true, m) { return m.try_lock(); } +inline bool tryEnterCS(Mutex& m) +SRT_TSA_NEEDS_NONLOCKED(m) +SRT_TSA_WILL_TRY_LOCK(true, m) +{ return m.try_lock(); } -inline void leaveCS(Mutex& m) SRT_ATTR_REQUIRES(m) SRT_ATTR_RELEASE(m) { m.unlock(); } +inline void leaveCS(Mutex& m) +SRT_TSA_NEEDS_LOCKED(m) +SRT_TSA_WILL_UNLOCK(m) +{ m.unlock(); } class InvertedLock { Mutex& m_mtx; public: - SRT_ATTR_REQUIRES(m) SRT_ATTR_RELEASE(m) + SRT_TSA_NEEDS_LOCKED(m) + SRT_TSA_WILL_UNLOCK(m) InvertedLock(Mutex& m) : m_mtx(m) { m_mtx.unlock(); } - SRT_ATTR_ACQUIRE(m_mtx) + SRT_TSA_WILL_LOCK(m_mtx) ~InvertedLock() { m_mtx.lock(); @@ -496,7 +506,7 @@ inline void releaseCond(Condition& cv) { cv.destroy(); } /// TODO: The class can be improved if needed to give writer a preference /// by adding additional m_iWritersWaiting member variable (counter). /// TODO: The m_iCountRead could be made atomic to make unlok_shared() faster and lock-free. -class SharedMutex +class SRT_TSA_CAPABILITY("mutex") SharedMutex { public: SharedMutex(); @@ -530,17 +540,17 @@ class SharedMutex /// A version of std::scoped_lock (or lock_guard for C++11). /// We could have used the srt::sync::ScopedLock making it a template-based class. /// But in that case all usages would have to be specificed like ScopedLock in C++03. -class SRT_ATTR_SCOPED_CAPABILITY ExclusiveLock +class SRT_TSA_SCOPED_CAPABILITY ExclusiveLock { public: - SRT_ATTR_ACQUIRE(m) + SRT_TSA_WILL_LOCK(m) explicit ExclusiveLock(SharedMutex& m) : m_mutex(m) { m_mutex.lock(); } - SRT_ATTR_RELEASE(m_mutex) + SRT_TSA_WILL_UNLOCK(m_mutex) ~ExclusiveLock() { m_mutex.unlock(); } private: @@ -548,17 +558,17 @@ class SRT_ATTR_SCOPED_CAPABILITY ExclusiveLock }; /// A reduced implementation of the std::shared_lock functionality (available in C++14). -class SRT_ATTR_SCOPED_CAPABILITY SharedLock +class SRT_TSA_SCOPED_CAPABILITY SharedLock { public: - SRT_ATTR_ACQUIRE_SHARED(m) + SRT_TSA_WILL_LOCK_SHARED(m) explicit SharedLock(SharedMutex& m) : m_mtx(m) { m_mtx.lock_shared(); } - SRT_ATTR_RELEASE_SHARED(m_mtx) + SRT_TSA_WILL_UNLOCK_SHARED(m_mtx) ~SharedLock() { m_mtx.unlock_shared(); } private: @@ -781,7 +791,7 @@ class CEvent // while having already the UniqueLock applied in the scope, // so a safe statement can be made about the mutex being locked // when signalling or waiting. -class CUniqueSync: public CSync +class SRT_TSA_SCOPED_CAPABILITY CUniqueSync: public CSync { UniqueLock m_ulock; @@ -789,20 +799,21 @@ class CUniqueSync: public CSync UniqueLock& locker() { return m_ulock; } - SRT_ATTR_ACQUIRE(this->m_ulock.mutex()) + SRT_TSA_WILL_LOCK(mut) CUniqueSync(Mutex& mut, Condition& cnd) : CSync(cnd, m_ulock) , m_ulock(mut) { } + SRT_TSA_WILL_LOCK(m_ulock.mutex()) CUniqueSync(CEvent& event) : CSync(event.cond(), m_ulock) , m_ulock(event.mutex()) { } - SRT_ATTR_RELEASE(this->m_ulock.mutex()) + SRT_TSA_WILL_UNLOCK(m_ulock.mutex()) ~CUniqueSync() {} // These functions can be used safely because diff --git a/testing/srt-test-live.cpp b/testing/srt-test-live.cpp index 1811220c8..12478e461 100644 --- a/testing/srt-test-live.cpp +++ b/testing/srt-test-live.cpp @@ -301,7 +301,7 @@ extern "C" int SrtCheckGroupHook(void* , SRTSOCKET acpsock, int , const sockaddr size = sizeof gt; if (-1 != srt_getsockflag(acpsock, SRTO_GROUPTYPE, >, &size)) { - if (gt < Size(gtypes)) + if (size_t(gt) < Size(gtypes)) Verb(" type=", gtypes[gt], VerbNoEOL); else Verb(" type=", int(gt), VerbNoEOL); diff --git a/testing/srt-test-multiplex.cpp b/testing/srt-test-multiplex.cpp index deb36554c..857557a5c 100644 --- a/testing/srt-test-multiplex.cpp +++ b/testing/srt-test-multiplex.cpp @@ -76,7 +76,7 @@ struct MediumPair bytevector initial_portion; string name; - MediumPair(unique_ptr s, unique_ptr t): src(move(s)), tar(move(t)) {} + MediumPair(unique_ptr s, unique_ptr t): src(std::move(s)), tar(std::move(t)) {} void Stop() { @@ -190,9 +190,9 @@ class MediaBase /// are still meant to be delivered to @c tar MediumPair& Link(std::unique_ptr src, std::unique_ptr tar, bytevector&& initial_portion, string name, string thread_name) { - media.emplace_back(move(src), move(tar)); + media.emplace_back(std::move(src), std::move(tar)); MediumPair& med = media.back(); - med.initial_portion = move(initial_portion); + med.initial_portion = std::move(initial_portion); med.name = name; // Ok, got this, so we can start transmission. @@ -382,7 +382,7 @@ bool SelectAndLink(SrtModel& m, string id, bool mode_output, string& w_msg) } bytevector dummy_initial_portion; - g_media_base.Link(move(source), move(target), move(dummy_initial_portion), os.str(), thread_name); + g_media_base.Link(std::move(source), std::move(target), std::move(dummy_initial_portion), os.str(), thread_name); return true; } diff --git a/testing/srt-test-relay.cpp b/testing/srt-test-relay.cpp index e7e5ae574..912af555e 100755 --- a/testing/srt-test-relay.cpp +++ b/testing/srt-test-relay.cpp @@ -320,7 +320,7 @@ SrtMainLoop::SrtMainLoop(const string& srt_uri, bool input_echoback, const strin Verb() << "Setting up output: " << spec; unique_ptr m { new TargetMedium }; m->Setup(Target::Create(spec)); - m_output_media.push_back(move(m)); + m_output_media.push_back(std::move(m)); } @@ -369,7 +369,7 @@ SrtMainLoop::SrtMainLoop(const string& srt_uri, bool input_echoback, const strin // Add SRT medium to output targets, and keep input medium empty. unique_ptr med { new TargetMedium }; med->Setup(m_srt_relay.get()); - m_output_media.push_back(move(med)); + m_output_media.push_back(std::move(med)); } else { diff --git a/testing/testmedia.cpp b/testing/testmedia.cpp index b9d8a0413..e6f5c871e 100755 --- a/testing/testmedia.cpp +++ b/testing/testmedia.cpp @@ -348,7 +348,7 @@ void SrtCommon::InitParameters(string host, string path, map par) } cc.token = token++; - m_group_nodes.push_back(move(cc)); + m_group_nodes.push_back(std::move(cc)); } par.erase("type"); @@ -3042,7 +3042,7 @@ extern unique_ptr CreateMedium(const string& uri) } if (ptr) - ptr->uri = move(u); + ptr->uri = std::move(u); return ptr; }