channel_fsm: fix rare deadlock

A deadlock may occur when ChannelFsm receives Event_stop_channel when
the non-blocking acquisition is in progress.

The control flow will be the following:

ChannelFsm::Event_stop_channel locks the mutex mx_
ChannelFsm::Event_stop_channel calls stop_acquisition
ChannelFsm::stop_acquisition calls PcpsAcquisitionAdapter::stop_acquisition
PcpsAcquisitionAdapter::stop_acquisition calls pcps_acquisition::set_active
pcps_acquisition::set_active calls pcps_acquisition::wait_if_active
pcps_acquisition::wait_if_active blocks waiting for the background
acquisition thread to finish processing
pcps_acquisition::acquisition_core calls pcps_acquisition::handle_threshold_reached
pcps_acquisition::handle_threshold_reached calls pcps_acquisition::send_positive_acquisition
pcps_acquisition::send_positive_acquisition calls ChannelFsm::Event_valid_acquisition
ChannelFsm::Event_valid_acquisition tries to lock the mutex mx_ for one
more time
Deadlock.

This commit does three things:
 - removes "magic numbers" used to mark different channel states as "magic numbers"
made the first attempt to fix the deadlock fail in a really strange way.
 - Implements a new state FSM_STATE_TERMINATING, used to prevent
unintended effects during FSM transition from tracking/acquisition to
FSM_STATE_IDLE.
 - Unlocks the mutex mx_ during actual attempt to stop
tracking/acquisition.

Signed-off-by: Vladislav P <vladisslav2011@gmail.com>
This commit is contained in:
Vladislav P
2026-09-18 02:31:50 +03:00
parent 96b4921f8c
commit a7ec48b46f
2 changed files with 34 additions and 23 deletions
+25 -22
View File
@@ -29,7 +29,7 @@
ChannelFsm::ChannelFsm()
: queue_(nullptr),
channel_(0U),
state_(0U)
state_(FSM_STATE_IDLE)
{
acq_ = nullptr;
trk_ = nullptr;
@@ -40,7 +40,7 @@ ChannelFsm::ChannelFsm(std::shared_ptr<AcquisitionInterface> acquisition)
: acq_(std::move(acquisition)),
queue_(nullptr),
channel_(0U),
state_(0U)
state_(FSM_STATE_IDLE)
{
trk_ = nullptr;
}
@@ -48,23 +48,26 @@ ChannelFsm::ChannelFsm(std::shared_ptr<AcquisitionInterface> acquisition)
bool ChannelFsm::Event_stop_channel()
{
std::lock_guard<std::mutex> lk(mx_);
std::unique_lock<std::mutex> lk(mx_);
auto current_state = state_;
state_ = FSM_STATE_TERMINATING;
DLOG(INFO) << "CH = " << channel_ << ". Ev stop channel";
switch (state_)
lk.unlock();
switch (current_state)
{
case 0: // already in stanby
break;
case 1: // acquisition
state_ = 0;
case FSM_STATE_IDLE:
return true;
case FSM_STATE_ACQUISITION:
stop_acquisition();
break;
case 2: // tracking
state_ = 0;
case FSM_STATE_TRACKING:
stop_tracking();
break;
default:
break;
}
lk.lock();
state_ = FSM_STATE_IDLE;
return true;
}
@@ -72,11 +75,11 @@ bool ChannelFsm::Event_stop_channel()
bool ChannelFsm::Event_start_acquisition_fpga()
{
std::lock_guard<std::mutex> lk(mx_);
if ((state_ == 1) || (state_ == 2))
if ((state_ == FSM_STATE_ACQUISITION) || (state_ == FSM_STATE_TRACKING) || (state_ == FSM_STATE_TERMINATING))
{
return false;
}
state_ = 1;
state_ = FSM_STATE_ACQUISITION;
DLOG(INFO) << "CH = " << channel_ << ". Ev start acquisition FPGA";
return true;
}
@@ -85,11 +88,11 @@ bool ChannelFsm::Event_start_acquisition_fpga()
bool ChannelFsm::Event_start_acquisition()
{
std::lock_guard<std::mutex> lk(mx_);
if ((state_ == 1) || (state_ == 2))
if ((state_ == FSM_STATE_ACQUISITION) || (state_ == FSM_STATE_TRACKING) || (state_ == FSM_STATE_TERMINATING))
{
return false;
}
state_ = 1;
state_ = FSM_STATE_ACQUISITION;
start_acquisition();
DLOG(INFO) << "CH = " << channel_ << ". Ev start acquisition";
return true;
@@ -99,11 +102,11 @@ bool ChannelFsm::Event_start_acquisition()
bool ChannelFsm::Event_valid_acquisition()
{
std::lock_guard<std::mutex> lk(mx_);
if (state_ != 1)
if (state_ != FSM_STATE_ACQUISITION)
{
return false;
}
state_ = 2;
state_ = FSM_STATE_TRACKING;
start_tracking();
DLOG(INFO) << "CH = " << channel_ << ". Ev valid acquisition";
return true;
@@ -113,11 +116,11 @@ bool ChannelFsm::Event_valid_acquisition()
bool ChannelFsm::Event_failed_acquisition_repeat()
{
std::lock_guard<std::mutex> lk(mx_);
if (state_ != 1)
if (state_ != FSM_STATE_ACQUISITION)
{
return false;
}
state_ = 1;
state_ = FSM_STATE_ACQUISITION;
start_acquisition();
DLOG(INFO) << "CH = " << channel_ << ". Ev failed acquisition repeat";
return true;
@@ -127,11 +130,11 @@ bool ChannelFsm::Event_failed_acquisition_repeat()
bool ChannelFsm::Event_failed_acquisition_no_repeat()
{
std::lock_guard<std::mutex> lk(mx_);
if (state_ != 1)
if (state_ != FSM_STATE_ACQUISITION)
{
return false;
}
state_ = 3;
state_ = FSM_STATE_FAILED_NO_REPEAT;
request_satellite();
DLOG(INFO) << "CH = " << channel_ << ". Ev failed acquisition no repeat";
return true;
@@ -141,11 +144,11 @@ bool ChannelFsm::Event_failed_acquisition_no_repeat()
bool ChannelFsm::Event_failed_tracking_standby()
{
std::lock_guard<std::mutex> lk(mx_);
if (state_ != 2)
if (state_ != FSM_STATE_TRACKING)
{
return false;
}
state_ = 0U;
state_ = FSM_STATE_IDLE;
notify_stop_tracking();
DLOG(INFO) << "CH = " << channel_ << ". Ev failed tracking standby";
return true;
+9 -1
View File
@@ -62,6 +62,14 @@ public:
virtual bool Event_failed_acquisition_no_repeat();
private:
enum fsm_state
{
FSM_STATE_IDLE = 0,
FSM_STATE_ACQUISITION,
FSM_STATE_TRACKING,
FSM_STATE_FAILED_NO_REPEAT,
FSM_STATE_TERMINATING
};
void start_tracking();
void stop_acquisition();
void stop_tracking();
@@ -77,7 +85,7 @@ private:
Concurrent_Queue<pmt::pmt_t>* queue_;
uint32_t channel_;
uint32_t state_;
fsm_state state_;
};