Blob Blame History Raw
From f40570f2f784dc61edb061a4931dcfc16bf51e7e Mon Sep 17 00:00:00 2001
From: Aravinda VK <avishwan@redhat.com>
Date: Mon, 5 Aug 2019 19:00:21 +0530
Subject: [PATCH 277/284] geo-rep: Fix Config Get Race

When two threads(sync jobs) in Geo-rep worker calls `gconf.get` and
`gconf.getr`(realtime) at the sametime, `getr` resets the conf object
and other one gets None. Thread Lock is introduced to fix the issue.

```
  File "/usr/libexec/glusterfs/python/syncdaemon/syncdutils.py",
  line 368, in twrap
    tf(*aargs)
  File "/usr/libexec/glusterfs/python/syncdaemon/master.py", line 1987,
  in syncjob
    po = self.sync_engine(pb, self.log_err)
  File "/usr/libexec/glusterfs/python/syncdaemon/resource.py",
  line 1444, in rsync
    rconf.ssh_ctl_args + \
AttributeError: 'NoneType' object has no attribute 'split'
```

Backport of:
 > Patch: https://review.gluster.org/#/c/glusterfs/+/23158/
 > Change-Id: I9c245e5c36338265354e158f5baa32b119eb2da5
 > Updates: bz#1737484
 > Signed-off-by: Aravinda VK <avishwan@redhat.com>

Change-Id: I9c245e5c36338265354e158f5baa32b119eb2da5
BUG: 1729915
Signed-off-by: Kotresh HR <khiremat@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/178960
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
 geo-replication/syncdaemon/gsyncdconfig.py | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/geo-replication/syncdaemon/gsyncdconfig.py b/geo-replication/syncdaemon/gsyncdconfig.py
index 1fc451f..38f3594 100644
--- a/geo-replication/syncdaemon/gsyncdconfig.py
+++ b/geo-replication/syncdaemon/gsyncdconfig.py
@@ -17,6 +17,7 @@ import os
 import shutil
 from string import Template
 from datetime import datetime
+from threading import Lock
 
 
 # Global object which can be used in other modules
@@ -35,6 +36,7 @@ class GconfInvalidValue(Exception):
 class Gconf(object):
     def __init__(self, default_conf_file, custom_conf_file=None,
                  args={}, extra_tmpl_args={}, override_from_args=False):
+        self.lock = Lock()
         self.default_conf_file = default_conf_file
         self.custom_conf_file = custom_conf_file
         self.tmp_conf_file = None
@@ -163,6 +165,11 @@ class Gconf(object):
         if value is not None and not self._is_valid_value(name, value):
             raise GconfInvalidValue()
 
+
+    def _load_with_lock(self):
+        with self.lock:
+            self._load()
+
     def _load(self):
         self.gconf = {}
         self.template_conf = []
@@ -230,12 +237,19 @@ class Gconf(object):
         self._tmpl_substitute()
         self._do_typecast()
 
-    def reload(self):
+    def reload(self, with_lock=True):
         if self._is_config_changed():
-            self._load()
+            if with_lock:
+                self._load_with_lock()
+            else:
+                self._load()
 
-    def get(self, name, default_value=None):
-        return self.gconf.get(name, default_value)
+    def get(self, name, default_value=None, with_lock=True):
+        if with_lock:
+            with self.lock:
+                return self.gconf.get(name, default_value)
+        else:
+            return self.gconf.get(name, default_value)
 
     def getall(self, show_defaults=False, show_non_configurable=False):
         cnf = {}
@@ -276,8 +290,9 @@ class Gconf(object):
         return cnf
 
     def getr(self, name, default_value=None):
-        self.reload()
-        return self.get(name, default_value)
+        with self.lock:
+            self.reload(with_lock=False)
+            return self.get(name, default_value, with_lock=False)
 
     def get_help(self, name=None):
         pass
-- 
1.8.3.1