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

Further mod on msvc config cache locking #4425

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

mwichmann
Copy link
Collaborator

In the first iteration, we took a read lock in the read case. A fail was observed where the reader took a json decode error, and then tried to remove the cachefile in case it's really corrupt. That failed as the file was "busy" - on Windows removing is essentially writing the file - so take a write lock here too.

This is a tweak to an internal detail that is not yet in an SCons release, so no further changes to CHANGES/RELEASE; no doc impacts. We don't have a test that can force this case, waiting for CI runs for more info.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt (and read the README.rst)
  • I have updated the appropriate documentation

In the first iteration, we took a read lock in the read case.
A fail was observed where the reader took a json decode error,
and then tried to remove the cachefile in case it's really corrupt.
That failed as the file was "busy" - on Windows removing is
essentially writing the file - so take a write lock here too.

Signed-off-by: Mats Wichmann <mats@linux.com>
@mwichmann mwichmann added the maintenance Tasks to maintain internal SCons code/tools label Oct 4, 2023
@bdbaddog bdbaddog merged commit 120b2cd into SCons:master Oct 5, 2023
3 of 5 checks passed
@mwichmann mwichmann deleted the main/scons-msvclock branch October 5, 2023 13:35
@mwichmann mwichmann added this to the 4.6 milestone Oct 5, 2023
@jcbrill
Copy link
Contributor

jcbrill commented Oct 18, 2023

When the file lock times-out, an SConsLockFailure exception is raised.

The SConsLockFailure is not handled in either the vc.py read_script_env_cache method (read cache) or the write_script_env_cache method (write cache).

I'm not sure the run should terminate if the lock can't be acquired. In both cases, it might be better to issue a warning and proceed without the cache.

An abnormal termination of scons can also orphan the lock file (i.e., created but not deleted) which creates problems for future runs (i.e., always time-out).

@mwichmann
Copy link
Collaborator Author

I'm a little worried if it's that slow that the lock times out, but yes. no effort yet to handle that case.

@jcbrill
Copy link
Contributor

jcbrill commented Oct 18, 2023

Ran into it this morning which was likely explained by a confluence of events that do not represent real world usage:

  • scons tests launched in a vwmare workstation from a vm network mapped drive folder on the host
  • the vm files themselves are located on an external usb 3.2(?) drive
  • anti-virus not disabled on the host pc or in the virtual pc
  • launched test after logging in to vm windows which apparently detected available updates and begin downloading

Probably hard to find a slower configuration. Unfortunately, the local tests pretty much all failed. After vm updated and restarted all was well again.

An orphaned lock file will always cause a run failure.

@mwichmann
Copy link
Collaborator Author

We can certainly add protocol to try to detect an orphaned lock file... I think the ninja daemon setup has something like this by using the psutil package. However, I'd like to avoid going down that route until it's more firmly established that ew need to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Tasks to maintain internal SCons code/tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants