Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/io/flutter/module/FlutterGeneratorPeer.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private void fillSdkCache() {
FlutterSdk sdk = FlutterSdk.forPath(path);
if (sdk != null) {
sdk.queryConfiguredPlatforms(false);
sdk.queryFlutterChannel(false);
sdk.queryFlutterChannel(false, myContext.getProject());
}
}
});
Expand Down
13 changes: 10 additions & 3 deletions src/io/flutter/sdk/FlutterSdk.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand All @@ -78,7 +79,7 @@ public class FlutterSdk {

private final @NotNull VirtualFile myHome;
private final @NotNull FlutterSdkVersion myVersion;
private final @NotNull Map<String, String> cachedConfigValues = new HashMap<>();
private final @NotNull Map<String, String> cachedConfigValues = new ConcurrentHashMap<>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

[MUST-FIX]: The switch to ConcurrentHashMap is a good improvement for thread safety. However, unlike HashMap, ConcurrentHashMap does not allow null values. In queryFlutterChannel (line 616), the branch variable (assigned from getLocation or parseChannel) could potentially be null, which would cause a NullPointerException when calling cachedConfigValues.put("channel", branch). Please ensure that a non-null default value (e.g., "unknown") is used if the result is null.


private FlutterSdk(@NotNull final VirtualFile home, @NotNull final FlutterSdkVersion version) {
myHome = home;
Expand Down Expand Up @@ -583,6 +584,12 @@ public String getDartSdkPath() {
@Nullable
@NonNls
public FlutterSdkChannel queryFlutterChannel(boolean useCachedValue) {
return queryFlutterChannel(useCachedValue, null);
}

@Nullable
@NonNls
public FlutterSdkChannel queryFlutterChannel(boolean useCachedValue, @Nullable Project project) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[NIT]: According to the Repository Style Guide (Rule 65), java.util.Optional should be used for return types that may be empty instead of returning null. Since this is a new overloaded method, consider returning Optional<FlutterSdkChannel> to align with modern Java practices in this codebase.

References
  1. Use java.util.Optional for return types that may be empty; avoid returning null. (link)

if (useCachedValue) {
final String channel = cachedConfigValues.get("channel");
Comment thread
helin24 marked this conversation as resolved.
if (channel != null) {
Expand All @@ -594,9 +601,9 @@ public FlutterSdkChannel queryFlutterChannel(boolean useCachedValue) {
assert dir != null;
String branch;
try {
branch = git4idea.light.LightGitUtilKt.getLocation(dir, GitExecutableManager.getInstance().getExecutable((Project)null));
branch = git4idea.light.LightGitUtilKt.getLocation(dir, GitExecutableManager.getInstance().getExecutable(project));
}
catch (VcsException e) {
catch (VcsException | IllegalStateException e) {
final String stdout = returnOutputOfQuery(flutterChannel());
if (stdout == null) {
branch = "unknown";
Expand Down
7 changes: 5 additions & 2 deletions src/io/flutter/sdk/FlutterSettingsConfigurable.java
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,11 @@ public void apply() throws ConfigurationException {
if (sdk != null) {
try {
lock.acquire();
sdk.queryFlutterChannel(false);
lock.release();
try {
sdk.queryFlutterChannel(false, myProject);
} finally {
lock.release();
}
}
catch (InterruptedException e) {
// do nothing
Expand Down
Loading