Skip to content

Commit c47b7e7

Browse files
azakkermanVladimir Kotal
authored andcommitted
Fix for #3307: Make isWorking synchronized to avoid read/write problems with mutable working field. Check whether git is working once per JVM run and don't recheck every time.
1 parent 0cbddc2 commit c47b7e7

File tree

2 files changed

+37
-26
lines changed

2 files changed

+37
-26
lines changed

opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.opengrok.indexer.util.BufferSink;
5050
import org.opengrok.indexer.util.Executor;
5151
import org.opengrok.indexer.util.HeadHandler;
52+
import org.opengrok.indexer.util.LazilyInstantiate;
5253
import org.opengrok.indexer.util.StringUtils;
5354
import org.opengrok.indexer.util.Version;
5455

@@ -95,6 +96,12 @@ public class GitRepository extends Repository {
9596
*/
9697
private static final Version MINIMUM_VERSION = new Version(2, 1, 2);
9798

99+
/**
100+
* This is a static replacement for 'working' field. Effectively, check if git is working once in a JVM
101+
* instead of calling it for every GitRepository instance.
102+
*/
103+
private static final LazilyInstantiate<Boolean> GIT_IS_WORKING = LazilyInstantiate.using(GitRepository::isGitWorking);
104+
98105
public GitRepository() {
99106
type = "git";
100107
/*
@@ -109,6 +116,21 @@ public GitRepository() {
109116
ignoredFiles.add(".git");
110117
}
111118

119+
private static boolean isGitWorking() {
120+
String repoCommand = getCommand(GitRepository.class, CMD_PROPERTY_KEY, CMD_FALLBACK);
121+
Executor exec = new Executor(new String[] {repoCommand, "--version"});
122+
if (exec.exec(false) == 0) {
123+
final String outputVersion = exec.getOutputString();
124+
final String version = outputVersion.replaceAll(".*? version (\\d+(\\.\\d+)*).*", "$1");
125+
try {
126+
return Version.from(version).compareTo(MINIMUM_VERSION) >= 0;
127+
} catch (NumberFormatException ex) {
128+
LOGGER.log(Level.WARNING, String.format("Unable to detect git version from %s", outputVersion), ex);
129+
}
130+
}
131+
return false;
132+
}
133+
112134
/**
113135
* Get an executor to be used for retrieving the history log for the named
114136
* file.
@@ -521,21 +543,7 @@ boolean isNestable() {
521543
@Override
522544
public boolean isWorking() {
523545
if (working == null) {
524-
ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK);
525-
Executor exec = new Executor(new String[]{RepoCommand, "--version"});
526-
527-
if (exec.exec(false) == 0) {
528-
final String outputVersion = exec.getOutputString();
529-
final String version = outputVersion.replaceAll(".*? version (\\d+(\\.\\d+)*).*", "$1");
530-
try {
531-
working = Version.from(version).compareTo(MINIMUM_VERSION) >= 0;
532-
} catch (NumberFormatException ex) {
533-
LOGGER.log(Level.WARNING, String.format("Unable to detect git version from %s", outputVersion), ex);
534-
working = false;
535-
}
536-
} else {
537-
working = false;
538-
}
546+
working = GIT_IS_WORKING.get();
539547
}
540548

541549
return working;

opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public Repository() {
117117

118118
/**
119119
* Gets the instance's repository command, primarily for testing purposes.
120-
* @return null if not {@link isWorking}, or otherwise a defined command
120+
* @return null if not {@link #isWorking()}, or otherwise a defined command
121121
*/
122122
public String getRepoCommand() {
123123
isWorking();
@@ -548,6 +548,17 @@ static Boolean checkCmd(String... args) {
548548
return exec.exec(false) == 0;
549549
}
550550

551+
protected static String getCommand(Class<? extends Repository> repoClass, String propertyKey, String fallbackCommand) {
552+
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
553+
String className = repoClass.getCanonicalName();
554+
String command = env.getRepoCmd(className);
555+
if (command == null) {
556+
command = System.getProperty(propertyKey, fallbackCommand);
557+
env.setRepoCmd(className, command);
558+
}
559+
return command;
560+
}
561+
551562
/**
552563
* Set the name of the external client command that should be used to access
553564
* the repository wrt. the given parameters. Does nothing, if this
@@ -561,18 +572,10 @@ static Boolean checkCmd(String... args) {
561572
* @see #RepoCommand
562573
*/
563574
protected String ensureCommand(String propertyKey, String fallbackCommand) {
564-
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
565-
566-
if (RepoCommand != null) {
567-
return RepoCommand;
568-
}
569-
570-
RepoCommand = env.getRepoCmd(this.getClass().getCanonicalName());
571575
if (RepoCommand == null) {
572-
RepoCommand = System.getProperty(propertyKey, fallbackCommand);
573-
env.setRepoCmd(this.getClass().getCanonicalName(), RepoCommand);
576+
RepoCommand = getCommand(this.getClass(), propertyKey, fallbackCommand);
574577
}
575-
578+
576579
return RepoCommand;
577580
}
578581

0 commit comments

Comments
 (0)