Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible race in CacheDir config #4489

Closed
mwichmann opened this issue Mar 8, 2024 · 5 comments · Fixed by #4490
Closed

Possible race in CacheDir config #4489

mwichmann opened this issue Mar 8, 2024 · 5 comments · Fixed by #4490
Labels

Comments

@mwichmann
Copy link
Collaborator

This was filed as a comment to closed PR #3353 by @mtvjr. Moving to a fresh issue because I believe it to be a different problem, plus that PR has been closed for over four years.

================

Looks like this might not completely make things thread safe. We just ran into an issue where it appears the cache file is attempted to be read between file creation and the json.dump call finishing. The json.load call failed due to an empty input file.

13:57:22  Traceback (most recent call last):
13:57:22    File "/usr/lib/python3.6/site-packages/SCons/CacheDir.py", line 195, in _readconfig
13:57:22      self.config = json.load(config)
13:57:22    File "/usr/lib64/python3.6/json/__init__.py", line 299, in load
13:57:22      parse_constant=parse_constant, object_pairs_hook=object_pairs_hook, **kw)
13:57:22    File "/usr/lib64/python3.6/json/__init__.py", line 354, in loads
13:57:22      return _default_decoder.decode(s)
13:57:22    File "/usr/lib64/python3.6/json/decoder.py", line 339, in decode
13:57:22      obj, end = self.raw_decode(s, idx=_w(s, 0).end())
13:57:22    File "/usr/lib64/python3.6/json/decoder.py", line 357, in raw_decode
13:57:22      raise JSONDecodeError("Expecting value", s, err.value) from None
13:57:22  json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
13:57:22  
13:57:22  During handling of the above exception, another exception occurred:
13:57:22  
13:57:22  Traceback (most recent call last):
13:57:22    File "/usr/lib/python3.6/site-packages/SCons/Taskmaster.py", line 230, in execute
13:57:22      if not t.retrieve_from_cache():
13:57:22    File "/usr/lib/python3.6/site-packages/SCons/Node/FS.py", line 2991, in retrieve_from_cache
13:57:22      return self.get_build_env().get_CacheDir().retrieve(self)
13:57:22    File "/usr/lib/python3.6/site-packages/SCons/Environment.py", line 1033, in get_CacheDir
13:57:22      cd = SCons.CacheDir.CacheDir(path)
13:57:22    File "/usr/lib/python3.6/site-packages/SCons/CacheDir.py", line 159, in __init__
13:57:22      self._readconfig(path)
13:57:22    File "/usr/lib/python3.6/site-packages/SCons/CacheDir.py", line 198, in _readconfig
13:57:22      raise SCons.Errors.SConsEnvironmentError(msg)
13:57:22  SCons.Errors.SConsEnvironmentError: Failed to read cache configuration for /var/lib/scons-ext/cache
@mwichmann mwichmann changed the title Possible race in cachedir config Possible race in CacheDir config Mar 8, 2024
@mwichmann mwichmann mentioned this issue Mar 8, 2024
3 tasks
@mwichmann
Copy link
Collaborator Author

Note the MSVC config caching recently has locking added, using the new FileLock class. We should be able to do the same for the CacheDir config.

@mtvjr
Copy link

mtvjr commented Mar 11, 2024

Thank you for the quick response and my apologies for not responding myself, I tend not to check GitHub very often. Let me know if you need any help or any additional information about the issue.

In our sconstruct, we've bypassed the issue by calling CacheDir directly as soon as we know what directory our cache will use, but before spawning any jobs.

@bdbaddog
Copy link
Contributor

Thank you for the quick response and my apologies for not responding myself, I tend not to check GitHub very often. Let me know if you need any help or any additional information about the issue.

In our sconstruct, we've bypassed the issue by calling CacheDir directly as soon as we know what directory our cache will use, but before spawning any jobs.

Are you spawning off other scons runs? or just regular build tasks?

@mwichmann
Copy link
Collaborator Author

Yeah, I'd say unless you're planning to have distinct CacheDir setups per environment, getting it set up early is good practice anyway.

mwichmann added a commit to mwichmann/scons that referenced this issue Mar 11, 2024
When creating a new CacheDir, the config file is created in
exclusive mode, but there's a timing window before the json dump
to the file completes when another thread could read the config
because it exists - but get a JSONDecodeError because it hasn't
finished writing yet.  Add locking so the readers will have to
wait until the writer is done.

Fixes SCons#4489

Signed-off-by: Mats Wichmann <mats@linux.com>
@mtvjr
Copy link

mtvjr commented Mar 13, 2024

Thank you for the quick response and my apologies for not responding myself, I tend not to check GitHub very often. Let me know if you need any help or any additional information about the issue.
In our sconstruct, we've bypassed the issue by calling CacheDir directly as soon as we know what directory our cache will use, but before spawning any jobs.

Are you spawning off other scons runs? or just regular build tasks?

It should just be regular build tasks, with custom actions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants