Skip to content

Commit

Permalink
Add comments to explain why no locking is necessary (#330)
Browse files Browse the repository at this point in the history
* Add comments to explain why no locking is necessary 

Add comments to explain why no locking is necessary in the MonitorObjects

* Add additional points

* Fix an inadvertent typo
  • Loading branch information
sramani-metro authored Dec 5, 2024
1 parent 6d71457 commit 0a17fe4
Showing 1 changed file with 37 additions and 1 deletion.
38 changes: 37 additions & 1 deletion Monitor/Monitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,8 @@ POP_WARNING()
}
inline void Open(PluginHost::IShell* service, Core::JSON::ArrayType<Config::Entry>::Iterator& index)
{
/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */

ASSERT((service != nullptr) && (_service == nullptr));

uint64_t baseTime = Core::Time::Now().Ticks();
Expand Down Expand Up @@ -730,6 +732,8 @@ POP_WARNING()
}
inline void Close()
{
/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */

ASSERT(_service != nullptr);

_job.Revoke();
Expand All @@ -740,6 +744,8 @@ POP_WARNING()
}
void Activated (const string& callsign, PluginHost::IShell* service) override
{
/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */

MonitorObjectContainer::iterator index(_monitor.find(callsign));

if (index != _monitor.end()) {
Expand All @@ -761,6 +767,8 @@ POP_WARNING()
}
void Deactivated (const string& callsign, PluginHost::IShell* service) override
{
/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */

MonitorObjectContainer::iterator index(_monitor.find(callsign));

if (index != _monitor.end()) {
Expand Down Expand Up @@ -793,6 +801,7 @@ POP_WARNING()
}
void Snapshot(Core::JSON::ArrayType<Monitor::Data>& snapshot) const
{
/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */
MonitorObjectContainer::const_iterator element(_monitor.cbegin());

// Go through the list of observations...
Expand All @@ -807,6 +816,7 @@ POP_WARNING()
}
bool Snapshot(const string& name, Monitor::MetaData& result, bool& operational) const
{
/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */
bool found = false;


Expand Down Expand Up @@ -848,7 +858,7 @@ POP_WARNING()

void Snapshot(const string& callsign, Core::JSON::ArrayType<JsonData::Monitor::InfoInfo>* response) const
{

/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */
ASSERT(response != nullptr);

if (callsign.empty() == false) {
Expand All @@ -865,6 +875,7 @@ POP_WARNING()

bool Reset(const string& name, Monitor::MetaData& result, bool& operational)
{
/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */
bool found = false;

MonitorObjectContainer::iterator index(_monitor.find(name));
Expand All @@ -881,6 +892,7 @@ POP_WARNING()

bool Reset(const string& name)
{
/* See comment in the Dispatch method on why no locking is here to protect the _monitor member */
bool found = false;

MonitorObjectContainer::iterator index(_monitor.find(name));
Expand All @@ -902,6 +914,30 @@ POP_WARNING()

void Dispatch()
{
/*
* The list _monitor in the MonitorObjects class is primarily modified in these scenarios:
*
* During the Open Call:
* The _monitor list is populated with objects when Open is called. New MonitorObject instances are constructed and added to _monitor using the emplace function.
*
* During the Close Call:
* The _monitor list is cleared in the Close method by invoking _monitor.clear().
*
* The list _monitor is accessed for read purposes (e.g., iterating over it, querying specific elements) in methods such as:
* Dispatch (evaluation logic)
* Snapshot (generating snapshots)
* Activated and Deactivated (modifying individual MonitorObject entries)
*
* It was made sure in the plugin Initialize/Deinitialze that the Activated/Deactivated notification
* cannot happen while Open/Close are called. It is important not to change the sequence of the code in Initialize/Deinitialize.
*
* Many of the member variables in MonitorObject (e.g., _nextSlot, _restartWindow, _active) are declared as std::atomic.
* These variables ensure thread-safe access without the need for explicit locking. Some of them are defined as const making them immutable.
*
* For the above reasons, It was determined that there is no need to use the adminLock to protect the MonitorObjects list even though the MonitorObjects are accessed from
* multiple threads.
* *
*/
uint64_t scheduledTime(Core::Time::Now().Ticks());
uint64_t nextSlot(static_cast<uint64_t>(~0));

Expand Down

0 comments on commit 0a17fe4

Please sign in to comment.