From 5d0d4e832eaaf5ffffbf933791750ccdf1cef71b Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Fri, 21 Dec 2018 12:09:26 +0100
Subject: [PATCH] Config, add more pessimistic locking option.

We usually only lock on write, but when data has a high posibility of changing in between reads, we should have an option to lock for writes exclusively.

needed for https://github.com/opnsense/core/issues/3062
---
 .../mvc/app/library/OPNsense/Core/Config.php  | 104 +++++++++++++-----
 1 file changed, 75 insertions(+), 29 deletions(-)

diff --git a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
index b093e6163..c361e945a 100644
--- a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
+++ b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
@@ -44,6 +44,12 @@ class Config extends Singleton
      */
     private $config_file = "";
 
+    /**
+     * config file handle
+     * @var null|file
+     */
+    private $config_file_handle = null;
+
     /**
      * SimpleXML type reference to config
      * @var SimpleXML
@@ -157,7 +163,9 @@ class Config extends Singleton
      */
     public function toArrayFromFile($filename, $forceList = null)
     {
-        $xml = $this->loadFromFile($filename);
+        $fp = fopen($filename, "r");
+        $xml = $this->loadFromStream($fp);
+        fclose($fp);
         return $this->toArray($forceList, $xml);
     }
 
@@ -298,18 +306,15 @@ class Config extends Singleton
     }
 
     /**
-     * load xml config from file
-     * @param string $filename config xml source
+     * load xml config from file handle
+     * @param file $fp config xml source
      * @return \SimpleXMLElement root node
      * @throws ConfigException when config could not be parsed
      */
-    private function loadFromFile($filename)
+    private function loadFromStream($fp)
     {
-        // exception handling
-        if (!file_exists($filename)) {
-            throw new ConfigException('file not found');
-        }
-        $xml = file_get_contents($filename);
+        fseek($fp, 0);
+        $xml = stream_get_contents($fp);
         if (trim($xml) == '') {
             throw new ConfigException('empty file');
         }
@@ -339,7 +344,17 @@ class Config extends Singleton
     {
         $this->simplexml = null;
         $this->statusIsValid = false;
-        $this->simplexml = $this->loadFromFile($this->config_file);
+
+        // exception handling
+        if (!file_exists($this->config_file)) {
+            throw new ConfigException('file not found');
+        }
+
+        if (!is_resource($this->config_file_handle)) {
+            $this->config_file_handle = fopen($this->config_file, "r+");
+        }
+
+        $this->simplexml = $this->loadFromStream($this->config_file_handle);
         $this->statusIsValid = true;
     }
 
@@ -488,6 +503,7 @@ class Config extends Singleton
         if ($this->isValid()) {
             // if current config is valid,
             $simplexml = $this->simplexml;
+            $config_file_handle = $this->config_file_handle;
             try {
                 // try to restore config
                 copy($filename, $this->config_file);
@@ -496,6 +512,7 @@ class Config extends Singleton
             } catch (ConfigException $e) {
                 // copy / load failed, restore previous version
                 $this->simplexml = $simplexml;
+                $this->config_file_handle = $config_file_handle;
                 $this->statusIsValid = true;
                 $this->save(null, true);
                 return false;
@@ -528,25 +545,54 @@ class Config extends Singleton
         // serialize to text
         $xml_text = $this->__toString();
 
-        // save configuration, try to obtain a lock before doing so.
-        $target_filename = $this->config_file;
-        if (file_exists($target_filename)) {
-            $fp = fopen($target_filename, "r+");
-        } else {
-            // apparently we are missing the config, not expected but open a new one.
-            $fp = fopen($target_filename, "w+");
-        }
-
-        if (flock($fp, LOCK_EX)) {
-            // lock aquired, truncate and write new data
-            ftruncate($fp, 0);
-            fwrite($fp, $xml_text);
-            // flush, unlock and close file handler
-            fflush($fp);
-            flock($fp, LOCK_UN);
-            fclose($fp);
-        } else {
-            throw new ConfigException("Unable to lock config");
+        if ($this->config_file_handle !== null) {
+            if (flock($this->config_file_handle, LOCK_EX)) {
+                fseek($this->config_file_handle, 0);
+                ftruncate($this->config_file_handle, 0);
+                fwrite($this->config_file_handle, $xml_text);
+                // flush, unlock, but keep the handle open
+                fflush($this->config_file_handle);
+                flock($this->config_file_handle, LOCK_UN);
+            } else {
+                throw new ConfigException("Unable to lock config");
+            }
         }
     }
+
+    /**
+     * cleanup, close file handle
+     */
+    public function __destruct ()
+    {
+        if ($this->config_file_handle !== null) {
+            fclose($this->config_file_handle);
+        }
+    }
+
+
+    /**
+     * lock configuration
+     * @param boolean $reload reload config from open file handle to enforce synchronicity
+     */
+    public function lock($reload=true)
+    {
+        if ($this->config_file_handle !== null) {
+            flock($this->config_file_handle, LOCK_EX);
+            if ($reload) {
+                $this->load();
+            }
+        }
+        return $this;
+    }
+
+    /**
+     * unlock configuration
+     */
+    public function unlock()
+    {
+        if (is_resource($this->config_file_handle)) {
+            flock($this->config_file_handle, LOCK_UN);
+        }
+        return $this;
+    }
 }