Replace use of CriticalSection in stat-leaseX-get commands with Memfile_LeaseMgr mutex lock
The command handlers for stat-leaseX-get commands create CriticalSections upon entry. We should be able to replace those with simply locking the Memfile lease manager's mutex like so:
tmark@wild-dog:stat_cmds (master) $ git diff
diff --git a/src/hooks/dhcp/stat_cmds/stat_cmds.cc b/src/hooks/dhcp/stat_cmds/stat_cmds.cc
index 8950bc928f..b9fe1800d2 100644
--- a/src/hooks/dhcp/stat_cmds/stat_cmds.cc
+++ b/src/hooks/dhcp/stat_cmds/stat_cmds.cc
@@ -718,7 +718,9 @@ int
StatCmds::statLease4GetHandler(CalloutHandle& handle) {
try {
LeaseStatCmdsImpl impl;
+#if 0
MultiThreadingCriticalSection cs;
+#endif
return (impl.statLease4GetHandler(handle));
} catch (const std::exception& ex) {
diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc
index afb3259840..86f95d2cd9 100644
--- a/src/lib/dhcpsrv/memfile_lease_mgr.cc
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc
@@ -1990,7 +1990,16 @@ Memfile_LeaseMgr::lfcExecute(boost::shared_ptr<LeaseFileType>& lease_file) {
LeaseStatsQueryPtr
Memfile_LeaseMgr::startLeaseStatsQuery4() {
LeaseStatsQueryPtr query(new MemfileLeaseStatsQuery4(storage4_));
+#if 1
+ if (MultiThreadingMgr::instance().getMode()) {
+ std::lock_guard<std::mutex> lock(*mutex_);
+ query->start();
+ } else {
+ query->start();
+ }
+#else
query->start();
+#endif
return(query);
}
I compiled with thread-sanitizer on Ubuntu 20.04, and ran kea-dhcp4 stand-alone in MT mode (8 threads), used traffic. I used a script to call socat locally to generate stat-lease4-get() at a rate every 100 ms. Each test started with an empty lease file and ran for sixty seconds. I tested three variations of the code 1: with CriticalSection 2: without CriticalSection or Mutex 3: With Mutex. The results are attached:
All three versions report one or two race warnings in StringSanitizerImpl, this is known false report. The version without CS or mutex had almost 90 race warnings emanating from alloc engine, which one would expect. The mutex-ed version is slightly faster than the CriticalSection version, and I suspect the difference would be larger if running HA+MT as a CS would cause both thread pools to pause/resume.
The stat commands do not need to create a CriticalSection and therefore should not. We might consider the same approach for the lease wipe commands.