Skip to content

Commit

Permalink
fix(stands): fix concurrency issues with stand assignments (#311)
Browse files Browse the repository at this point in the history
There are potential crash points due to lack of concurrency checking.

fix #310
  • Loading branch information
AndyTWF committed Aug 6, 2021
1 parent b309c37 commit 35b9e2a
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
17 changes: 12 additions & 5 deletions src/plugin/stands/StandEventHandler.cpp
Expand Up @@ -75,6 +75,7 @@ namespace UKControllerPlugin {
[](unsigned char c) { return std::toupper(c); }
);

auto mapLock = this->LockStandMap();
std::string callsign = aircraft->GetCallsign();
// If no stand is selected, delete the current assignment if there is one
if (
Expand Down Expand Up @@ -121,11 +122,6 @@ namespace UKControllerPlugin {
}
}

size_t StandEventHandler::CountStands(void) const
{
return this->stands.size();
}

size_t StandEventHandler::CountStandAssignments(void) const
{
return this->standAssignments.size();
Expand Down Expand Up @@ -258,6 +254,7 @@ namespace UKControllerPlugin {

// If a stand is assigned, set the starting text to be the stand identifier
std::string startingText = "";
auto mapLock = this->LockStandMap();
auto assignedStand = this->standAssignments.find(flightplan.GetCallsign());
if (
assignedStand != this->standAssignments.cend() &&
Expand Down Expand Up @@ -294,6 +291,7 @@ namespace UKControllerPlugin {
*/
void StandEventHandler::SetTagItemData(UKControllerPlugin::Tag::TagData& tagData)
{
auto mapLock = this->LockStandMap();
if (
this->standAssignments.count(tagData.flightPlan.GetCallsign())
) {
Expand Down Expand Up @@ -373,6 +371,7 @@ namespace UKControllerPlugin {
EuroScopeCFlightPlanInterface& flightPlan,
EuroScopeCRadarTargetInterface& radarTarget
) {
auto mapLock = this->LockStandMap();
if (!this->standAssignments.count(flightPlan.GetCallsign())) {
return;
}
Expand Down Expand Up @@ -413,6 +412,7 @@ namespace UKControllerPlugin {
}

// Delete all existing assignments
auto mapLock = this->LockStandMap();
this->standAssignments.clear();

for (
Expand Down Expand Up @@ -444,6 +444,7 @@ namespace UKControllerPlugin {
*/
void StandEventHandler::ProcessPushEvent(const Push::PushEvent& message)
{
auto mapLock = this->LockStandMap();
if (message.event == "App\\Events\\StandAssignedEvent") {
// If a stand has been assigned, assign it here
if (!AssignmentMessageValid(message.data)) {
Expand All @@ -455,6 +456,7 @@ namespace UKControllerPlugin {
message.data.at("callsign").get<std::string>(),
message.data.at("stand_id").get<int>()
);

this->standAssignments[message.data.at("callsign").get<std::string>()] =
message.data.at("stand_id").get<int>();
LogInfo(
Expand Down Expand Up @@ -485,5 +487,10 @@ namespace UKControllerPlugin {
}
};
}

std::lock_guard<std::mutex> StandEventHandler::LockStandMap()
{
return std::lock_guard(this->mapMutex);
}
} // namespace Stands
} // namespace UKControllerPlugin
11 changes: 7 additions & 4 deletions src/plugin/stands/StandEventHandler.h
Expand Up @@ -12,12 +12,12 @@
namespace UKControllerPlugin {
namespace Stands {
/*
A new class
Handles events related to stands.
*/
class StandEventHandler: public UKControllerPlugin::Tag::TagItemInterface,
public Push::PushEventProcessorInterface,
public UKControllerPlugin::Flightplan::FlightPlanEventHandlerInterface,
public UKControllerPlugin::Integration::ExternalMessageHandlerInterface
public UKControllerPlugin::Flightplan::FlightPlanEventHandlerInterface,
public UKControllerPlugin::Integration::ExternalMessageHandlerInterface
{
public:
StandEventHandler(
Expand All @@ -36,7 +36,6 @@ namespace UKControllerPlugin {
std::string identifier
);

size_t CountStands(void) const;
size_t CountStandAssignments(void) const;
void DisplayStandSelectionMenu(
UKControllerPlugin::Euroscope::EuroScopeCFlightPlanInterface& flightplan,
Expand Down Expand Up @@ -105,6 +104,7 @@ namespace UKControllerPlugin {
std::string GetAirfieldForStandAssignment(
UKControllerPlugin::Euroscope::EuroScopeCFlightPlanInterface& flightplan
);
std::lock_guard<std::mutex> LockStandMap();

// The last airfield that was used to populate the stand menu, used when we receive the callback
std::string lastAirfieldUsed = "";
Expand All @@ -123,6 +123,9 @@ namespace UKControllerPlugin {

// The currently assigned stands and who they are assigned to
std::map<std::string, int> standAssignments;

// Locks the stand assignments map to prevent concurrent edits
std::mutex mapMutex;
};
} // namespace Stands
} // namespace UKControllerPlugin

0 comments on commit 35b9e2a

Please sign in to comment.