dl_harq: fix unlocking of unlocked mutex bug detected by TSAN

in the mac_test the tb_decoded() method was called twice for
the 2nd codeword, causig TSAN to complain about an unlocked mutex
being unlocked.

The patch resolves the potential issue only calling tb_decoded
for a grant/tb thas has a non-zero MCS.

The patch also adjusts the reset function to have a safe and "unsafe"
version to be called from inside the class, similar to other
classes where we do the same.
master
Andre Puschmann 4 years ago
parent 5c55ff24ec
commit 018c734b09

@ -63,7 +63,7 @@ private:
~dl_tb_process(); ~dl_tb_process();
bool init(int pid, dl_harq_entity* parent, uint32_t tb_idx); bool init(int pid, dl_harq_entity* parent, uint32_t tb_idx);
void reset(bool lock = true); void reset();
void reset_ndi(); void reset_ndi();
void new_grant_dl(mac_interface_phy_lte::mac_grant_dl_t grant, mac_interface_phy_lte::tb_action_dl_t* action); void new_grant_dl(mac_interface_phy_lte::mac_grant_dl_t grant, mac_interface_phy_lte::tb_action_dl_t* action);
@ -73,6 +73,9 @@ private:
// Determine if it's a new transmission 5.3.2.2 // Determine if it's a new transmission 5.3.2.2
bool calc_is_new_transmission(mac_interface_phy_lte::mac_grant_dl_t grant); bool calc_is_new_transmission(mac_interface_phy_lte::mac_grant_dl_t grant);
// Internal function to reset process, caller must hold the mutex
void reset_unsafe();
std::mutex mutex; std::mutex mutex;
bool is_initiated; bool is_initiated;

@ -149,7 +149,9 @@ void dl_harq_entity::dl_harq_process::tb_decoded(mac_interface_phy_lte::mac_gran
{ {
/* For each subprocess... */ /* For each subprocess... */
for (uint32_t i = 0; i < SRSRAN_MAX_TB; i++) { for (uint32_t i = 0; i < SRSRAN_MAX_TB; i++) {
subproc[i].tb_decoded(grant, &ack[i]); if (grant.tb[i].tbs) {
subproc[i].tb_decoded(grant, &ack[i]);
}
} }
} }
@ -196,12 +198,14 @@ bool dl_harq_entity::dl_harq_process::dl_tb_process::init(int pid, dl_harq_entit
return true; return true;
} }
void dl_harq_entity::dl_harq_process::dl_tb_process::reset(bool lock) void dl_harq_entity::dl_harq_process::dl_tb_process::reset()
{ {
if (lock) { std::lock_guard<std::mutex> lock(mutex);
mutex.lock(); reset_unsafe();
} }
void dl_harq_entity::dl_harq_process::dl_tb_process::reset_unsafe()
{
bzero(&cur_grant, sizeof(mac_interface_phy_lte::mac_grant_dl_t)); bzero(&cur_grant, sizeof(mac_interface_phy_lte::mac_grant_dl_t));
is_first_tb = true; is_first_tb = true;
ack = false; ack = false;
@ -213,10 +217,6 @@ void dl_harq_entity::dl_harq_process::dl_tb_process::reset(bool lock)
} }
payload_buffer_ptr = NULL; payload_buffer_ptr = NULL;
} }
if (lock) {
mutex.unlock();
}
} }
void dl_harq_entity::dl_harq_process::dl_tb_process::reset_ndi() void dl_harq_entity::dl_harq_process::dl_tb_process::reset_ndi()
@ -293,7 +293,8 @@ void dl_harq_entity::dl_harq_process::dl_tb_process::new_grant_dl(mac_interface_
n_retx, n_retx,
n_retx > RESET_DUPLICATE_TIMEOUT ? "yes" : "no"); n_retx > RESET_DUPLICATE_TIMEOUT ? "yes" : "no");
if (n_retx > RESET_DUPLICATE_TIMEOUT) { if (n_retx > RESET_DUPLICATE_TIMEOUT) {
reset(false); // reset without trying to acquire the mutex again
reset_unsafe();
} }
} }
@ -351,11 +352,12 @@ void dl_harq_entity::dl_harq_process::dl_tb_process::tb_decoded(mac_interface_ph
cur_grant.tb[tid].ndi); cur_grant.tb[tid].ndi);
} }
mutex.unlock();
if (ack && is_bcch) { if (ack && is_bcch) {
reset(); // reset without trying to acquire the mutex again
reset_unsafe();
} }
mutex.unlock();
} }
// Determine if it's a new transmission 5.3.2.2 // Determine if it's a new transmission 5.3.2.2

Loading…
Cancel
Save