aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPau Espin Pedrol <pespin@sysmocom.de>2020-07-09 17:39:20 +0200
committerPau Espin Pedrol <pespin@sysmocom.de>2020-07-10 17:32:03 +0200
commit8b0c5368f53b16a00206b64f319ff08cdf32d521 (patch)
treedbd7a139d44f3434e5a76094278628a16dfdb61a
parentc0d6fd27ff4c6cfa57303a37bc452f87c56e0b6c (diff)
Transceiver: Fix race condition obtaining Dl burst from Upper layer
The queue was being accessed sequentially obtaining and releasing the mutual exclusion zone twice. First in getStaleBurst() dropping all FN<currTime, then in getCurrentBurst() trying to obtain FN=currTime. However, since in between the mutex is released, it could happen that for instance upper layer would introduce currTime-1 in the queue, which would make then getCurrentBurst() detect that one instead of potential currTime in the queue and return NULL. By holding the mutex during the call to both functions we make sure the state is kept during the whole transaction. Related: OS#4487 (comment #7) Change-Id: If1fd8d7fc5f21ee2894192ef1ac2a3cdda6bbb98
-rw-r--r--Transceiver52M/Transceiver.cpp23
-rw-r--r--Transceiver52M/radioVector.cpp16
-rw-r--r--Transceiver52M/radioVector.h1
3 files changed, 17 insertions, 23 deletions
diff --git a/Transceiver52M/Transceiver.cpp b/Transceiver52M/Transceiver.cpp
index d45a28b..7aaf1d4 100644
--- a/Transceiver52M/Transceiver.cpp
+++ b/Transceiver52M/Transceiver.cpp
@@ -433,9 +433,15 @@ void Transceiver::pushRadioVector(GSM::Time &nowTime)
std::vector<bool> filler(mChans, true);
bool stale_bursts_changed;
+ TN = nowTime.TN();
+
for (size_t i = 0; i < mChans; i ++) {
state = &mStates[i];
stale_bursts_changed = false;
+ zeros[i] = state->chanType[TN] == NONE;
+
+ Mutex *mtx = mTxPriorityQueues[i].getMutex();
+ mtx->lock();
while ((burst = mTxPriorityQueues[i].getStaleBurst(nowTime))) {
LOGCHAN(i, DTRXDDL, NOTICE) << "dumping STALE burst in TRX->SDR interface ("
@@ -447,15 +453,6 @@ void Transceiver::pushRadioVector(GSM::Time &nowTime)
delete burst;
}
- if (stale_bursts_changed)
- dispatch_trx_rate_ctr_change(state, i);
-
- TN = nowTime.TN();
- modFN = nowTime.FN() % state->fillerModulus[TN];
-
- bursts[i] = state->fillerTable[modFN][TN];
- zeros[i] = state->chanType[TN] == NONE;
-
if ((burst = mTxPriorityQueues[i].getCurrentBurst(nowTime))) {
bursts[i] = burst->getVector();
@@ -467,7 +464,15 @@ void Transceiver::pushRadioVector(GSM::Time &nowTime)
}
delete burst;
+ } else {
+ modFN = nowTime.FN() % state->fillerModulus[TN];
+ bursts[i] = state->fillerTable[modFN][TN];
}
+
+ mtx->unlock();
+
+ if (stale_bursts_changed)
+ dispatch_trx_rate_ctr_change(state, i);
}
mRadioInterface->driveTransmitRadio(bursts, zeros);
diff --git a/Transceiver52M/radioVector.cpp b/Transceiver52M/radioVector.cpp
index ad40a11..68e42c5 100644
--- a/Transceiver52M/radioVector.cpp
+++ b/Transceiver52M/radioVector.cpp
@@ -120,38 +120,26 @@ GSM::Time VectorQueue::nextTime() const
radioVector* VectorQueue::getStaleBurst(const GSM::Time& targTime)
{
- mLock.lock();
- if ((mQ.size()==0)) {
- mLock.unlock();
+ if ((mQ.size()==0))
return NULL;
- }
if (mQ.top()->getTime() < targTime) {
radioVector* retVal = mQ.top();
mQ.pop();
- mLock.unlock();
return retVal;
}
- mLock.unlock();
-
return NULL;
}
radioVector* VectorQueue::getCurrentBurst(const GSM::Time& targTime)
{
- mLock.lock();
- if ((mQ.size()==0)) {
- mLock.unlock();
+ if ((mQ.size()==0))
return NULL;
- }
if (mQ.top()->getTime() == targTime) {
radioVector* retVal = mQ.top();
mQ.pop();
- mLock.unlock();
return retVal;
}
- mLock.unlock();
-
return NULL;
}
diff --git a/Transceiver52M/radioVector.h b/Transceiver52M/radioVector.h
index 0a14a4d..84e3987 100644
--- a/Transceiver52M/radioVector.h
+++ b/Transceiver52M/radioVector.h
@@ -65,6 +65,7 @@ public:
GSM::Time nextTime() const;
radioVector* getStaleBurst(const GSM::Time& targTime);
radioVector* getCurrentBurst(const GSM::Time& targTime);
+ Mutex *getMutex() const { return &mLock; };
};
#endif /* RADIOVECTOR_H */