From 48387e92a09effbae0b8c925ca5ba615cb112b76 Mon Sep 17 00:00:00 2001 From: Peter Razumovsky Date: Fri, 4 Mar 2016 17:04:28 +0300 Subject: [PATCH] Fix race condition in reload_on_sighup functional Race condition appears in next situation: 1. First thread calls _set_config_value for one section. 2. Second thread calls _set_config_value for another section. 3. First thread. Config option value set, calls open(self.config_file, 'wb'), which erases all file content. 4. Second thread. In previous point moment second thread tries to set config option value to self.config_file, which is empty (see 3). So, NoSectionError exception raised. This patch adds ten retries for setting option value, if NoSectionError raised, i.e. try to wait until self.config_file is busy. Change-Id: Ic54ea287ebe4724511f75d42677cae5dfdec4e57 Closes-bug: #1535766 --- heat_integrationtests/common/config.py | 5 ++++ .../functional/test_reload_on_sighup.py | 23 +++++++++++++++++-- .../heat_integrationtests.conf.sample | 5 ++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/heat_integrationtests/common/config.py b/heat_integrationtests/common/config.py index f8ae075d37..3aee48f00c 100644 --- a/heat_integrationtests/common/config.py +++ b/heat_integrationtests/common/config.py @@ -130,6 +130,11 @@ IntegrationTestGroup = [ default=30, help="Timeout in seconds to wait for adding or removing child" "process after receiving of sighup signal"), + cfg.IntOpt('sighup_config_edit_retries', + default=10, + help='Count of retries to edit config file during sighup. If ' + 'another worker already edit config file, file can be ' + 'busy, so need to wait and try edit file again.'), cfg.StrOpt('heat-config-notify-script', default=('heat-config-notify'), help="Path to the script heat-config-notify"), diff --git a/heat_integrationtests/functional/test_reload_on_sighup.py b/heat_integrationtests/functional/test_reload_on_sighup.py index cba5386264..b014f49c2b 100644 --- a/heat_integrationtests/functional/test_reload_on_sighup.py +++ b/heat_integrationtests/functional/test_reload_on_sighup.py @@ -28,8 +28,27 @@ class ReloadOnSighupTest(functional_base.FunctionalTestsBase): def _set_config_value(self, service, key, value): config = configparser.ConfigParser() - config.read(self.config_file) - config.set(service, key, value) + + # NOTE(prazumovsky): If there are several workers, there can be + # situation, when one thread opens self.config_file for writing + # (so config_file erases with opening), in that moment other thread + # intercepts to this file and try to set config option value, i.e. + # write to file, which is already erased by first thread, so, + # NoSectionError raised. So, should wait until first thread writes to + # config_file. + retries_count = self.conf.sighup_config_edit_retries + while True: + config.read(self.config_file) + try: + config.set(service, key, value) + except configparser.NoSectionError: + if retries_count <= 0: + raise + retries_count -= 1 + eventlet.sleep(1) + else: + break + with open(self.config_file, 'wb') as f: config.write(f) diff --git a/heat_integrationtests/heat_integrationtests.conf.sample b/heat_integrationtests/heat_integrationtests.conf.sample index 160a275bb3..ba37d79e08 100644 --- a/heat_integrationtests/heat_integrationtests.conf.sample +++ b/heat_integrationtests/heat_integrationtests.conf.sample @@ -112,5 +112,10 @@ # receiving of sighup signal (integer value) #sighup_timeout = 30 +# Count of retries to edit config file during sighup. If another worker already +# edit config file, file can be busy, so need to wait and try edit file +# again. (integer value) +#sighup_config_edit_retries = 10 + # Path to the script heat-config-notify (string value) #heat_config_notify_script = heat-config-notify