[Resource Leak Analysis] NPE and build failure

eclipse-jdt#3328
stephan-herrmann · Dec 29, 2024 · d773ff0 · d773ff0
1 parent 347d85b
commit d773ff0
Show file tree

Hide file tree

Showing 2 changed files with 149 additions and 8 deletions.
diff --git a/....core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java b/....core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java
@@ -576,16 +576,14 @@ private static FakedTrackingVariable getMoreUnsafeFromBranches(ConditionalExpres
 	{
 		FakedTrackingVariable trackerIfTrue = retriever.apply(conditionalExpression.valueIfTrue);
 		FakedTrackingVariable trackerIfFalse = retriever.apply(conditionalExpression.valueIfFalse);
-		if (trackerIfTrue == null)
-			return trackerIfFalse;
-		if (trackerIfFalse == null)
-			return trackerIfTrue;
 		return pickMoreUnsafe(trackerIfTrue, trackerIfFalse, flowInfo);
 	}
 
 	private static FakedTrackingVariable pickMoreUnsafe(FakedTrackingVariable tracker1, FakedTrackingVariable tracker2, FlowInfo info) {
 		// whichever of the two trackers has stronger indication to be leaking will be returned,
 		// the other one will be removed from the scope (considered to be merged into the former).
+		if (tracker1 == null) return tracker2;
+		if (tracker2 == null) return tracker1;
 		int status1 = info.nullStatus(tracker1.binding);
 		int status2 = info.nullStatus(tracker2.binding);
 		if (status1 == FlowInfo.NULL || status2 == FlowInfo.NON_NULL) return pick(tracker1, tracker2);
@@ -905,10 +903,7 @@ else if (expression instanceof CastExpression)
 			for (Expression result : se.resultExpressions()) {
 				FakedTrackingVariable current = analyseCloseableExpression(scope, flowInfo, flowContext, useAnnotations,
 						local, location, result, previousTracker);
-				if (mostRisky == null)
-					mostRisky = current;
-				else
-					mostRisky = pickMoreUnsafe(mostRisky, current, flowInfo);
+				mostRisky = pickMoreUnsafe(mostRisky, current, flowInfo);
 			}
 			return mostRisky;
 		}

diff --git a/....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java b/....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java
@@ -7343,4 +7343,150 @@ String getBug561334_log() {
 	----------
 	""";
 }
+public void testGH3328() {
+	if (this.complianceLevel < ClassFileConstants.JDK14) return;
+	runConformTest(
+		new String[] {
+			"org/example/ExampleService.java",
+			"""
+			package org.example;
+
+			import java.util.ArrayList;
+			import java.util.List;
+
+			class StationNode {
+				public String getPath() {
+					return "abcdefghijklmn";
+				}
+			}
+			class RadioChannel implements AutoCloseable {
+
+				@Override
+				public void close() throws Exception {
+				}
+			}
+
+			public class ExampleService {
+
+				private RadioChannel createRadioChannel() {
+					return new RadioChannel();
+				}
+
+				public ArrayList<RadioChannel> compilationFails(List<StationNode> nodes) {
+					ArrayList<RadioChannel> channels = new ArrayList<>(6);
+					for (StationNode node : nodes) {
+						var entryName = node.getPath();
+						RadioChannel channel = switch (entryName) {
+						case "A", "B" -> createRadioChannel();
+						case "C", "D" -> createRadioChannel();
+						default -> null;
+						};
+						if (channel != null) {
+							channels.add(channel);
+						}
+					}
+					return channels;
+				}
+
+				public ArrayList<RadioChannel> compilationOk1(List<StationNode> nodes) {
+					ArrayList<RadioChannel> channels = new ArrayList<>(6);
+					for (StationNode node : nodes) {
+						var entryName = node.getPath();
+						RadioChannel channel = switch (entryName) {
+						case "A", "B" -> new RadioChannel();
+						case "C", "D" -> new RadioChannel();
+						default -> null;
+						};
+						if (channel != null) {
+							channels.add(channel);
+						}
+					}
+					return channels;
+				}
+
+				public ArrayList<RadioChannel> compilesOk2(List<StationNode> nodes) {
+					ArrayList<RadioChannel> channels = new ArrayList<>(6);
+					for (StationNode node : nodes) {
+						var entryName = node.getPath();
+
+						RadioChannel channel;
+						switch(entryName) {
+						case "A":
+						case "B":
+							channel = createRadioChannel();
+							break;
+						case "C":
+						case "D":
+							channel = createRadioChannel();
+							break;
+						default:
+							channel = null;
+						}
+						if (channel != null) {
+							channels.add(channel);
+						}
+					}
+					return channels;
+				}
+
+			}
+			"""
+		});
+}
+public void testGH3328_2() {
+	if (this.complianceLevel < ClassFileConstants.JDK14) return;
+	runConformTest(
+		new String[] {
+			"org/example/ExampleService.java",
+			"""
+			package org.example;
+
+			import java.util.ArrayList;
+			import java.util.List;
+
+			interface SomeInterface {
+				void someMethod();
+			}
+			class StationNode {
+				public String getPath() {
+					return "abcdefghijklmn";
+				}
+			}
+			class RadioChannel implements SomeInterface, AutoCloseable {
+
+				@Override
+				public void close() throws Exception {
+				}
+
+				@Override
+				public void someMethod() {
+				}
+
+			}
+
+			public class ExampleService {
+
+				private RadioChannel createRadioChannel() {
+					return new RadioChannel();
+				}
+
+				public ArrayList<RadioChannel> compilationFails(List<StationNode> nodes) {
+					ArrayList<RadioChannel> channels = new ArrayList<>(6);
+					for (StationNode node : nodes) {
+						var entryName = node.getPath();
+						RadioChannel channel = switch (entryName) {
+						case "A", "B" -> createRadioChannel();
+						case "C", "D" -> createRadioChannel();
+						default -> null;
+						};
+						if (channel != null) {
+							channels.add(channel);
+						}
+					}
+					return channels;
+				}
+			}
+			"""
+		});
+}
 }