From f40570f2f784dc61edb061a4931dcfc16bf51e7e Mon Sep 17 00:00:00 2001 From: Aravinda VK 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 Change-Id: I9c245e5c36338265354e158f5baa32b119eb2da5 BUG: 1729915 Signed-off-by: Kotresh HR Reviewed-on: https://code.engineering.redhat.com/gerrit/178960 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- 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