Captive portal: use locking to avoid race conditions between rc.prunecaptiveportal and captiveportal_disconnect_all()

Convert rc.prunecaptiveportal to lock()/unlock()/try_lock() and use the lock to ensure that there aren't race conditions between it and captiveportal_disconnect_all().

(cherry picked from commit d793617ee9b4c3f66575737df3e8f6cf04e7c782)

2 files changed, 20 insertions, 10 deletions

diff --git a/src/etc/inc/captiveportal.inc b/src/etc/inc/captiveportal.inc
index c404f47..02ec3d8 100644
--- a/src/etc/inc/captiveportal.inc
+++ b/src/etc/inc/captiveportal.inc
@@ -999,6 +999,16 @@ function captiveportal_disconnect_client($sessionid, $term_cause = 1, $logoutRea
 function captiveportal_disconnect_all($term_cause = 6, $logoutReason = "DISCONNECT") {
 	global $g, $config, $cpzone, $cpzoneid;
 
+	/* check if we're pruning old entries and eventually wait */
+	$rcprunelock = try_lock("rcprunecaptiveportal{$cpzone}", 60);
+
+	/* if we still don't have the lock, unlock forcefully and take it */
+	if (!$rcprunelock) {
+		log_error("CP zone ${cpzone}: could not obtain the lock for more than 60 seconds, lock taken forcefully to disconnect all users");
+		unlock_force("rcprunecaptiveportal{$cpzone}");
+		$rcprunelock = lock("rcprunecaptiveportal{$cpzone}", LOCK_EX);
+	}
+
 	$radiussrvs = captiveportal_get_radius_servers();
 	$cpdb = captiveportal_read_db();
 
@@ -1018,6 +1028,8 @@ function captiveportal_disconnect_all($term_cause = 6, $logoutReason = "DISCONNE
 		captiveportal_logportalauth($cpentry[4], $cpentry[3], $cpentry[2], $logoutReason);
 	}
 	unset($cpdb);
+
+	unlock($rcprunelock);
 }
 
 /* send RADIUS acct stop for all current clients */
diff --git a/src/etc/rc.prunecaptiveportal b/src/etc/rc.prunecaptiveportal
index 8e639d9..56cc7f9 100755
--- a/src/etc/rc.prunecaptiveportal
+++ b/src/etc/rc.prunecaptiveportal
@@ -75,18 +75,16 @@ if (!is_array($config['captiveportal'][$cpzone])) {
 }
 $cpzoneid = $config['captiveportal'][$cpzone]['zoneid'];
 
-if (file_exists("{$g['tmp_path']}/.rc.prunecaptiveportal.{$cpzone}.running")) {
-	$stat = stat("{$g['tmp_path']}/.rc.prunecaptiveportal.{$cpzone}.running");
-	if (time() - $stat['mtime'] >= 120) {
-		@unlink("{$g['tmp_path']}/.rc.prunecaptiveportal.{$cpzone}.running");
-	} else {
-		log_error("Skipping CP pruning process because previous/another instance is already running");
-		return;
-	}
+$rcprunelock = try_lock("rcprunecaptiveportal{$cpzone}", 3);
+
+if (!$rcprunelock) {
+	log_error("Skipping CP pruning process for zone {$cpzone} because previous/another instance is already running");
+	unlock_force("rcprunecaptiveportal{$cpzone}");
+	return;
 }
 
-@file_put_contents("{$g['tmp_path']}/.rc.prunecaptiveportal.{$cpzone}.running", "");
 captiveportal_prune_old();
-@unlink("{$g['tmp_path']}/.rc.prunecaptiveportal.{$cpzone}.running");
+
+unlock($rcprunelock);
 
 ?>