Skip to content

Commit

Permalink
Fix CommandLine class initialization deadlock.
Browse files Browse the repository at this point in the history
#21566 (comment) shows a deadlock when initializing static variable `CommandLine.EMPTY` and another thread is initializing `CommandLine`'s subclass `AbstractCommandLine` because `CommandLine.EMPTY` is an instance of `AbstractCommandLine`.

Fixes #21566.

Closes #21608.

PiperOrigin-RevId: 613736999
Change-Id: Iadb738b0a89f34cee9a03bc92b385fa7ab58c98f
  • Loading branch information
benjaminp authored and copybara-github committed Mar 8, 2024
1 parent eb09906 commit ae71ce9
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@
/** A representation of a list of arguments. */
public abstract class CommandLine {

public static final CommandLine EMPTY = new EmptyCommandLine();
public static CommandLine empty() {
return EmptyCommandLine.INSTANCE;
}

/** Returns a {@link CommandLine} backed by the given list of arguments. */
public static CommandLine of(ImmutableList<String> arguments) {
return arguments.isEmpty() ? CommandLine.EMPTY : new SimpleCommandLine(arguments);
return arguments.isEmpty() ? empty() : new SimpleCommandLine(arguments);
}

/**
Expand All @@ -40,7 +42,7 @@ public static CommandLine concat(CommandLine commandLine, ImmutableList<String>
if (args.isEmpty()) {
return commandLine;
}
if (commandLine == EMPTY) {
if (commandLine == EmptyCommandLine.INSTANCE) {
return CommandLine.of(args);
}
return new SuffixedCommandLine(args, commandLine);
Expand Down Expand Up @@ -126,6 +128,8 @@ public abstract void addToFingerprint(
throws CommandLineExpansionException, InterruptedException;

private static final class EmptyCommandLine extends AbstractCommandLine {
private static final EmptyCommandLine INSTANCE = new EmptyCommandLine();

@Override
public ImmutableList<String> arguments() {
return ImmutableList.of();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,10 +526,10 @@ private static CommandLine computeArgs(RuleContext ruleContext) throws Interrupt
if (!ruleContext.getRule().isAttrDefined("args", Type.STRING_LIST)) {
// Some non-_binary rules create RunfilesSupport instances; it is fine to not have an args
// attribute here.
return CommandLine.EMPTY;
return CommandLine.empty();
}
ImmutableList<String> args = ruleContext.getExpander().withDataLocations().tokenized("args");
return args.isEmpty() ? CommandLine.EMPTY : CommandLine.of(args);
return args.isEmpty() ? CommandLine.empty() : CommandLine.of(args);
}

private static ActionEnvironment computeActionEnvironment(RuleContext ruleContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ Builder addFormatted(Object object, String format) {

CommandLine build(boolean flagPerLine) {
if (arguments.isEmpty()) {
return CommandLine.EMPTY;
return CommandLine.empty();
}
Object[] args = arguments.toArray();
return flagPerLine
Expand Down

0 comments on commit ae71ce9

Please sign in to comment.