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

Fix race condition and add more logging for null entry error message #21668

Closed

Conversation

oquenchil
Copy link
Contributor

Attempts to address NPE reported in: bazelbuild/bazel-skylib#488 (comment) and #21665 (comment)

The put() call to the runfiles dir map is placed before the call that stashes the corresponding directory to address the race condition described here: bazelbuild/bazel-skylib#488 (comment).

The exception will now log:

  • entries in the runfiles dir map
  • environment variables
  • stashes on disk

@oquenchil oquenchil requested a review from fmeum March 13, 2024 09:58
@github-actions github-actions bot added team-Local-Exec Issues and PRs for the Execution (Local) team awaiting-review PR is awaiting review from an assigned reviewer labels Mar 13, 2024
@@ -154,11 +173,11 @@ private void stashSandboxInternal(
path.getRelative("execroot/" + environment.get("TEST_WORKSPACE") + "/_tmp"));
}
}
path.getChild("execroot").renameTo(stashPathExecroot);
if (isTestAction(mnemonic)) {
// We only do this after the rename operation has succeeded
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is no longer correct.

@meisterT
Copy link
Member

@bazel-io fork 7.1.1

@meisterT
Copy link
Member

@bazel-io fork 7.1.1

@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Mar 14, 2024
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Mar 14, 2024
Attempts to address NPE reported in: bazelbuild/bazel-skylib#488 (comment) and bazelbuild#21665 (comment)

The `put()` call to the runfiles dir map is placed before the call that stashes the corresponding directory to address the race condition described here: bazelbuild/bazel-skylib#488 (comment).

The exception will now log:
- entries in the runfiles dir map
- environment variables
- stashes on disk

Closes bazelbuild#21668.

PiperOrigin-RevId: 615739651
Change-Id: Ida90e334d1d1f890cf204d272134726bb1f70eb9
github-merge-queue bot pushed a commit that referenced this pull request Mar 14, 2024
…message (#21692)

Attempts to address NPE reported in:
bazelbuild/bazel-skylib#488 (comment)
and
#21665 (comment)

The `put()` call to the runfiles dir map is placed before the call that
stashes the corresponding directory to address the race condition
described here:
bazelbuild/bazel-skylib#488 (comment).

The exception will now log:
- entries in the runfiles dir map
- environment variables
- stashes on disk

Closes #21668.

Commit
59dbf7a

PiperOrigin-RevId: 615739651
Change-Id: Ida90e334d1d1f890cf204d272134726bb1f70eb9

Co-authored-by: Pedro <plf@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Local-Exec Issues and PRs for the Execution (Local) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants