Bobby Bruce has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/57789 )

Change subject: stdlib: Add file lock to the resources.json download
......................................................................

stdlib: Add file lock to the resources.json download

There have been failures on the weekly tests during the decoding of the
downloaded resources.json base64 file. These errors suggested an
incomplete download or some form of file corruption. These errors only
ever seem to occur when multiple threads of gem5 are running. It has
therefore been proposed that perhaps, in some cases, the cached
downloaded file was bring re-downloaded while also being read by
another thread. For this reason this patch adds a filelock so only one
instance of gem5, at any one time, can download and read the
resources.json file. Even if this is not the cause of the weekly test
errors, it still adds some additional safeguards.

Change-Id: I7c6e1c1786c1919e8519587e53b6a77f4aafa932
---
M src/python/gem5/resources/downloader.py
1 file changed, 46 insertions(+), 19 deletions(-)



diff --git a/src/python/gem5/resources/downloader.py b/src/python/gem5/resources/downloader.py
index 5ca7387..828ebc8 100644
--- a/src/python/gem5/resources/downloader.py
+++ b/src/python/gem5/resources/downloader.py
@@ -83,27 +83,34 @@
         f"gem5-resources-{hashlib.md5(url.encode()).hexdigest()}.base64",
     )

- # The resources.json file can change at any time, but to avoid excessive - # retrieval we cache a version locally and use it for up to an hour before
-    # obtaining a fresh copy.
-    #
- # `time.time()` and `os.path.getmtime(..)` both return an unix epoch time
-    # in seconds. Therefore, the value of "3600" here represents an hour
- # difference between the two values. `time.time()` gets the current time, - # and `os.path.getmtime(<file>)` gets the modification time of the file. - # This is the most portable solution as other ideas, like "file creation
-    # time", are  not always the same concept between operating systems.
-    if not use_caching or not os.path.exists(file_path) or \
-        (time.time() - os.path.getmtime(file_path)) > 3600:
-                _download(url, file_path)
+    # We apply a lock on the resources file for when it's downloaded, or
+    # re-downloaded, and read. This stops a corner-case from occuring where
+    # the file is re-downloaded while being read by another gem5 thread.
+ # Note the timeout is 120 so the `_download` function is given time to run
+    # its Truncated Exponential Backoff algorithm
+    # (maximum of roughly 1 minute). Typically this code will run quickly.
+    with FileLock("{}.lock".format(file_path), timeout=120):

-    # Note: Google Source does not properly support obtaining files as raw
-    # text. Therefore when we open the URL we receive the JSON in base64
-    # format. Conversion is needed before it can be loaded.
-    with open(file_path) as file:
- to_return = json.loads(base64.b64decode(file.read()).decode("utf-8"))
+        # The resources.json file can change at any time, but to avoid
+ # excessive retrieval we cache a version locally and use it for up to
+        # an hour before obtaining a fresh copy.
+        #
+ # `time.time()` and `os.path.getmtime(..)` both return an unix epoch + # time in seconds. Therefore, the value of "3600" here represents an
+        # hour difference between the two values. `time.time()` gets the
+ # current time, and `os.path.getmtime(<file>)` gets the modification + # time of the file. This is the most portable solution as other ideas, + # like "file creation time", are not always the same concept between
+        # operating systems.
+        if not use_caching or not os.path.exists(file_path) or \
+            (time.time() - os.path.getmtime(file_path)) > 3600:
+                    _download(url, file_path)

-    return to_return
+ # Note: Google Source does not properly support obtaining files as raw + # text. Therefore when we open the URL we receive the JSON in base64
+        # format. Conversion is needed before it can be loaded.
+        with open(file_path) as file:
+ return json.loads(base64.b64decode(file.read()).decode("utf-8"))

 def _get_resources_json() -> Dict:
     """

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/57789
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I7c6e1c1786c1919e8519587e53b6a77f4aafa932
Gerrit-Change-Number: 57789
Gerrit-PatchSet: 1
Gerrit-Owner: Bobby Bruce <bbr...@ucdavis.edu>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to