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