Skip to content

Commit

Permalink
Fix potential class initialization deadlocks
Browse files Browse the repository at this point in the history
By ensuring that nested classes that extend the containing class, and are
referenced by the containing class' static initializer, cannot be initialized
from outside the containing class.

Without this change, deadlocks can occur if the nested and containing classes
are initialized by different threads, e.g.:

```
SpawnCache.NO_CACHE // thread 1
new ContainingPackageLookupValue.NoContainingPackage("") // thread 2
```

or:

```
new NoSpawnCache() // thread 1
ContainingPackageLookupValue.NONE // thread 2
```

PiperOrigin-RevId: 615919227
Change-Id: I3c513fa349c878cc604d2862a233e33a503c8293
  • Loading branch information
cushon authored and copybara-github committed Mar 14, 2024
1 parent 4efe725 commit 589237b
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ public static class NoSpawnCache implements SpawnCache {
public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) {
return SpawnCache.NO_RESULT_NO_STORE;
}

private NoSpawnCache() {}
}

/** A no-op implementation that has no results and performs no stores. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
}

if (ErrorReason.REPOSITORY_NOT_FOUND.equals(pkgLookupValue.getErrorReason())) {
return new ContainingPackageLookupValue.NoContainingPackage(pkgLookupValue.getErrorMsg());
return ContainingPackageLookupValue.noContainingPackage(pkgLookupValue.getErrorMsg());
}
PathFragment parentDir = dir.getPackageFragment().getParentDirectory();
if (parentDir == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ public static ContainingPackage withContainingPackage(PackageIdentifier pkgId, R
return new ContainingPackage(pkgId, root);
}

static ContainingPackageLookupValue noContainingPackage(String reason) {
return new NoContainingPackage(reason);
}

/** Value indicating there is no containing package. */
public static class NoContainingPackage extends ContainingPackageLookupValue {
private final String reason;
Expand All @@ -147,7 +151,7 @@ private NoContainingPackage() {
this.reason = null;
}

NoContainingPackage(@Nonnull String reason) {
private NoContainingPackage(@Nonnull String reason) {
this.reason = reason;
}

Expand Down

0 comments on commit 589237b

Please sign in to comment.