Repository: AgnostiqHQ/covalent (python)

Issue:
Corrupted config file
### Environment

- **Covalent version**: 0.198.0
- **Python version**: 3.8.13
- **Operating system**: MacOS 12.3.1


### What is happening?

It seems the corrupted config file issue has crept up again. This is most likely due to performing multiple writes from different threads.

### How can we reproduce the issue?

After installing covalent, when I do

```bash
covalent start --ignore-migrations -d
covalent purge -y
```

Then the following error is thrown:
```bash
toml.decoder.TomlDecodeError: Found invalid character in key name: ']'. Try quoting the key name. (line 33 column 16 char 927)
```

This happens even when say a test is run after starting covalent server. So it is rather sporadic, and might be difficult to reproduce.

### What should happen?

No error should be thrown.

### Any suggestions?

I'd say the solution for this would be to not perform any writes whenever the `ConfigManager` is initialized. Instead do it when necessary. Reading however can still be performed.

Pull request:
Lock config file during config updates
<!--
⚠️ Your pull request title should be short detailed and understandable for all.
⚠️ Increment the version number in the /VERSION file. If this is a bugfix, increment the patch version (the rightmost number), for example 0.18.2 becomes 0.18.3. If it's a new feature, increment the minor version (the middle number), for example 0.18.2 becomes 0.19.0.
⚠️ Add a note to /CHANGELOG.md with your version number and the date, summarizing the changes.
⚠️ If your pull request fixes an open issue, please link to the issue.
⚠️ Rebase the latest changes from the develop branch. Assuming origin points to https://github.com/AgnostiqHQ/covalent, you should run git rebase origin/develop.
-->

- [x] I have added the tests to cover my changes.
- [x] I have updated the documentation and CHANGELOG accordingly.
- [x] I have read the CONTRIBUTING document.

Closes #1235.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7c2770fa..0f94bd8c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `requirements-client.txt` file added.
 
+### Fixed
+
+- Config file is now locked during reads and writes to mitigate concurrency issues
+
 ### Changed
 
 - Installation requirements are now split into client side and server side requirements' files.
diff --git a/covalent/_shared_files/config.py b/covalent/_shared_files/config.py
index 47523d52..89eb82e9 100644
--- a/covalent/_shared_files/config.py
+++ b/covalent/_shared_files/config.py
@@ -18,6 +18,7 @@
 #
 # Relief from the License may be granted by purchasing a commercial license.
 
+import fcntl
 import os
 import shutil
 from dataclasses import asdict
@@ -47,10 +48,13 @@ class ConfigManager:
         self.config_file = DEFAULT_CONFIG["sdk"]["config_file"]
 
         self.generate_default_config()
-        if os.path.exists(self.config_file):
-            # Update config with user configuration file:
+
+        try:
+            with open(self.config_file, "r") as f:
+                pass
+
             self.update_config()
-        else:
+        except FileNotFoundError:
             Path(self.config_file).parent.mkdir(parents=True, exist_ok=True)
             self.write_config()
 
@@ -110,14 +114,16 @@ class ConfigManager:
                         else:
                             old_dict.setdefault(key, value)
 
-        if os.path.exists(self.config_file):
-            file_config = toml.load(self.config_file)
+        with open(self.config_file, "r+") as f:
+            fcntl.lockf(f, fcntl.LOCK_EX)
+            file_config = toml.load(f)
+
             update_nested_dict(self.config_data, file_config)
             if new_entries:
                 update_nested_dict(self.config_data, new_entries, override_existing)
 
-        # Writing it back to the file
-        self.write_config()
+            # Writing it back to the file
+            self.write_config()
 
     def read_config(self) -> None:
         """
@@ -142,8 +148,8 @@ class ConfigManager:
         Returns:
             None
         """
-
         with open(self.config_file, "w") as f:
+            fcntl.lockf(f, fcntl.LOCK_EX)
             toml.dump(self.config_data, f)
 
     def purge_config(self) -> None:


Test patch:
diff --git a/tests/covalent_tests/shared_files/config_test.py b/tests/covalent_tests/shared_files/config_test.py
index 83dc08ea..f1b8bdcc 100644
--- a/tests/covalent_tests/shared_files/config_test.py
+++ b/tests/covalent_tests/shared_files/config_test.py
@@ -77,7 +77,8 @@ def test_config_manager_init_write_update_config(
             "covalent._shared_files.config.ConfigManager.get", side_effect=config_keys
         )
         path_mock = mocker.patch("covalent._shared_files.config.Path.__init__", return_value=None)
-        mocker.patch("os.path.exists", return_value=path_exists)
+        side_effect = None if path_exists else FileNotFoundError
+        open_mock = mocker.patch("covalent._shared_files.config.open", side_effect=side_effect)
 
         cm = ConfigManager()
         assert hasattr(cm, "config_data")
@@ -209,10 +210,12 @@ def test_write_config(mocker):
     cm = ConfigManager()
     toml_dump_mock = mocker.patch("covalent._shared_files.config.toml.dump")
     open_mock = mocker.patch("covalent._shared_files.config.open")
+    lock_mock = mocker.patch("fcntl.lockf")
     mock_file = open_mock.return_value.__enter__.return_value
     cm.write_config()
     toml_dump_mock.assert_called_once_with(cm.config_data, mock_file)
     open_mock.assert_called_once_with(cm.config_file, "w")
+    lock_mock.assert_called_once()
 
 
 def test_config_manager_set(mocker):