Merge pull request #26584 Backport of fixes to 7.6.3

Fixes
* #25802
* #25781

Co-authored-by: Louis Jacomet <louis@gradle.com>

subprojects/core/src/main/java/org/gradle/execution/plan/FinalizerGroup.java

@@ -22,6 +22,7 @@
 import com.google.common.collect.SetMultimap;
 
 import javax.annotation.Nullable;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
@@ -43,6 +44,7 @@ public class FinalizerGroup extends HasFinalizers {
     @Nullable
     private ElementSuccessors successors;
     private boolean finalizedNodeHasStarted;
+    private boolean hasBeenScheduled;
 
     public FinalizerGroup(TaskNode node, NodeGroup delegate) {
         this.ordinal = delegate.asOrdinal();
@@ -159,6 +161,12 @@ public void onNodeStart(Node finalizer, Node node) {
     }
 
     public void scheduleMembers(SetMultimap<FinalizerGroup, FinalizerGroup> reachableGroups) {
+        if (hasBeenScheduled) {
+            return;
+        } else {
+            hasBeenScheduled = true;
+        }
+
         Set<Node> finalizedNodesToBlockOn = findFinalizedNodesThatDoNotIntroduceACycle(reachableGroups);
         WaitForNodesToComplete waitForFinalizers = new WaitForNodesToComplete(finalizedNodesToBlockOn);
 
@@ -176,6 +184,10 @@ public void scheduleMembers(SetMultimap<FinalizerGroup, FinalizerGroup> reachabl
         // There are some finalized nodes that are also members
         // For each member, determine which finalized nodes to wait for
         ImmutableMap.Builder<Node, MemberSuccessors> blockingNodesBuilder = ImmutableMap.builder();
+
+        // Calculate the set of dependencies of finalized nodes that are also members of this group
+        Set<Node> dependenciesThatAreMembers = getDependenciesThatAreMembers(blockedFinalizedMembers);
+
         for (Node member : members) {
             if (isFinalizerNode(member) || memberCanStartAtAnyTime(member)) {
                 // Short-circuit for these, they are handled separately
@@ -190,22 +202,15 @@ public void scheduleMembers(SetMultimap<FinalizerGroup, FinalizerGroup> reachabl
                     blockingNodesBuilder.put(member, new WaitForFinalizedNodesToBecomeActive(Collections.singleton(member)));
                 }
             } else {
-                // Determine whether this member is a dependency of a finalized node, in which case treat it as if it were a finalized member
-                boolean requiredByBlockedFinalizedMember = false;
-                for (Node finalizedMember : blockedFinalizedMembers) {
-                    if (dependsOn(finalizedMember, member)) {
-                        requiredByBlockedFinalizedMember = true;
-                        break;
-                    }
-                }
-                if (requiredByBlockedFinalizedMember) {
+                if (dependenciesThatAreMembers.contains(member)) {
+                    // This member is a dependency of a finalized member. Treat is as if it were a finalized member.
                     blockingNodesBuilder.put(member, waitForFinalizers);
-                    continue;
+                } else {
+                    // Wait for the finalized nodes that don't introduce a cycle
+                    Set<Node> blockOn = new LinkedHashSet<>(finalizedNodesToBlockOn);
+                    blockOn.addAll(blockedFinalizedMembers);
+                    blockingNodesBuilder.put(member, new WaitForNodesToComplete(blockOn));
                 }
-                // Wait for the finalized nodes that don't introduce a cycle
-                Set<Node> blockOn = new LinkedHashSet<>(finalizedNodesToBlockOn);
-                blockOn.addAll(blockedFinalizedMembers);
-                blockingNodesBuilder.put(member, new WaitForNodesToComplete(blockOn));
             }
         }
         ImmutableMap<Node, MemberSuccessors> blockingNodes = blockingNodesBuilder.build();
@@ -260,21 +265,26 @@ private MemberSuccessors getNodesThatBlock(Node node) {
         return successors.getNodesThatBlock(node);
     }
 
-    private boolean dependsOn(Node fromNode, Node toNode) {
-        Set<Node> seen = new HashSet<>();
-        List<Node> queue = new ArrayList<>();
-        Iterables.addAll(queue, fromNode.getHardSuccessors());
-        while (!queue.isEmpty()) {
-            Node node = queue.remove(0);
-            if (node == toNode) {
-                return true;
-            }
-            if (!seen.add(node)) {
-                continue;
+    private Set<Node> getDependenciesThatAreMembers(Set<Node> blockedFinalizedMembers) {
+        Set<Node> dependenciesThatAreMembers = new HashSet<>(members.size());
+        Set<Node> seen = new HashSet<>(1024);
+
+        ArrayDeque<Node> queue = new ArrayDeque<>(1024);
+        for (Node fromNode : blockedFinalizedMembers) {
+            queue.add(fromNode);
+            while (!queue.isEmpty()) {
+                Node toNode = queue.removeFirst();
+                if (members.contains(toNode) && !blockedFinalizedMembers.contains(toNode)) {
+                    dependenciesThatAreMembers.add(toNode);
+                }
+                toNode.visitHardSuccessors(node -> {
+                    if (seen.add(node)) {
+                        queue.add(node);
+                    }
+                });
             }
-            Iterables.addAll(queue, node.getHardSuccessors());
         }
-        return false;
+        return dependenciesThatAreMembers;
     }
 
     private boolean hasACycle(Node finalized, SetMultimap<FinalizerGroup, FinalizerGroup> reachableGroups) {

subprojects/core/src/main/java/org/gradle/execution/plan/Node.java

@@ -535,6 +535,16 @@ public Iterable<Node> getHardSuccessors() {
         return dependencyNodes.getDependencySuccessors();
     }
 
+    /**
+     * Iterates over the nodes which are hard successors applying a visitor, i.e. which have a non-removable relationship to the current node.
+     *
+     * This can be a more efficient way to iterate over the successors than {@link #getHardSuccessors()}.
+     */
+    @OverridingMethodsMustInvokeSuper
+    public void visitHardSuccessors(Consumer<? super Node> visitor) {
+        dependencyNodes.getDependencySuccessors().forEach(visitor);
+    }
+
     public SortedSet<Node> getFinalizers() {
         return dependentNodes.getFinalizers();
     }

subprojects/core/src/main/java/org/gradle/execution/plan/TaskNode.java

@@ -22,6 +22,7 @@
 
 import java.util.NavigableSet;
 import java.util.Set;
+import java.util.function.Consumer;
 
 import static org.gradle.execution.plan.NodeSets.newSortedNodeSet;
 
@@ -92,6 +93,13 @@ public Iterable<Node> getHardSuccessors() {
         );
     }
 
+    @Override
+    public void visitHardSuccessors(Consumer<? super Node> visitor) {
+        finalizingSuccessors.forEach(visitor);
+        getMustSuccessors().forEach(visitor);
+        super.visitHardSuccessors(visitor);
+    }
+
     public abstract TaskInternal getTask();
 
     protected void deprecateLifecycleHookReferencingNonLocalTask(String hookName, Node taskNode) {

subprojects/core/src/test/groovy/org/gradle/execution/plan/DefaultExecutionPlanTest.groovy

@@ -457,7 +457,7 @@ class DefaultExecutionPlanTest extends AbstractExecutionPlanSpec {
         orderingRule << ['dependsOn', 'mustRunAfter', 'shouldRunAfter']
     }
 
-    def "finalizer groups that finalize each other form a cycle"() {
+    def "finalizer groups that finalize each other do not form a cycle"() {
         given:
         TaskInternal finalizerA = createTask("finalizerA")
         TaskInternal finalizerB = createTask("finalizerB")

subprojects/process-services/src/main/java/org/gradle/process/internal/health/memory/CGroupMemoryInfo.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2022 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.gradle.process.internal.health.memory;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.io.Files;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.Charset;
+
+public class CGroupMemoryInfo implements OsMemoryInfo {
+    private static final String CGROUP_MEM_USAGE_FILE = "/sys/fs/cgroup/memory/memory.usage_in_bytes";
+    private static final String CGROUP_MEM_TOTAL_FILE = "/sys/fs/cgroup/memory/memory.limit_in_bytes";
+
+    @Override
+    public OsMemoryStatus getOsSnapshot() {
+        String memUsageString = readStringFromFile(new File(CGROUP_MEM_USAGE_FILE));
+        String memTotalString = readStringFromFile(new File(CGROUP_MEM_TOTAL_FILE));
+
+        return getOsSnapshotFromCgroup(memUsageString, memTotalString);
+    }
+
+    private String readStringFromFile(File file) {
+        try {
+            return Files.asCharSource(file, Charset.defaultCharset()).readFirstLine();
+        } catch (IOException e) {
+            throw new UnsupportedOperationException("Unable to read system memory from " + file.getAbsoluteFile(), e);
+        }
+    }
+
+    @VisibleForTesting
+    OsMemoryStatusSnapshot getOsSnapshotFromCgroup(String memUsageString, String memTotalString) {
+        long memUsage;
+        long memTotal;
+        long memAvailable;
+
+        try {
+            memUsage = Long.parseLong(memUsageString);
+            memTotal = Long.parseLong(memTotalString);
+            memAvailable = Math.max(0, memTotal - memUsage);
+        } catch (NumberFormatException e) {
+            throw new UnsupportedOperationException("Unable to read system memory", e);
+        }
+
+        return new OsMemoryStatusSnapshot(memTotal, memAvailable);
+    }
+}

subprojects/process-services/src/main/java/org/gradle/process/internal/health/memory/DefaultOsMemoryInfo.java

@@ -26,12 +26,37 @@ public DefaultOsMemoryInfo() {
         if (operatingSystem.isMacOsX()) {
             delegate = new NativeOsMemoryInfo();
         } else if (operatingSystem.isLinux()) {
-            delegate = new MemInfoOsMemoryInfo();
+            delegate = getLinuxDelegate();
         } else {
             delegate = new MBeanOsMemoryInfo();
         }
     }
 
+    private OsMemoryInfo getLinuxDelegate() {
+        OsMemoryInfo cGroupDelegate = new CGroupMemoryInfo();
+        OsMemoryInfo memInfoDelegate = new MemInfoOsMemoryInfo();
+
+        OsMemoryStatus cGroupSnapshot;
+        OsMemoryStatus memInfoSnapshot;
+
+        try {
+            cGroupSnapshot = cGroupDelegate.getOsSnapshot();
+        } catch (UnsupportedOperationException e) {
+            return memInfoDelegate;
+        }
+
+        try {
+            memInfoSnapshot = memInfoDelegate.getOsSnapshot();
+        } catch (UnsupportedOperationException e) {
+            return cGroupDelegate;
+        }
+
+        long cGroupFreeMemory = cGroupSnapshot.getFreePhysicalMemory();
+        long memInfoFreeMemory = memInfoSnapshot.getFreePhysicalMemory();
+
+        return cGroupFreeMemory > memInfoFreeMemory ? memInfoDelegate : cGroupDelegate;
+    }
+
     @Override
     public OsMemoryStatus getOsSnapshot() {
         return delegate.getOsSnapshot();

subprojects/process-services/src/main/java/org/gradle/process/internal/health/memory/MemInfoOsMemoryInfo.java

@@ -47,11 +47,8 @@ public synchronized OsMemoryStatus getOsSnapshot() {
         } catch (IOException e) {
             throw new UnsupportedOperationException("Unable to read system memory from " + MEMINFO_FILE_PATH, e);
         }
-        OsMemoryStatusSnapshot memInfo = getOsSnapshotFromMemInfo(meminfoOutputLines);
-        if (memInfo.getFreePhysicalMemory() < 0 || memInfo.getTotalPhysicalMemory() < 0) {
-            throw new UnsupportedOperationException("Unable to read system memory from " + MEMINFO_FILE_PATH);
-        }
-        return memInfo;
+
+        return getOsSnapshotFromMemInfo(meminfoOutputLines);
     }
 
 
@@ -80,6 +77,10 @@ OsMemoryStatusSnapshot getOsSnapshotFromMemInfo(final List<String> meminfoLines)
             }
         }
 
+        if (meminfo.getAvailable() < 0 || meminfo.getTotal() < 0) {
+            throw new UnsupportedOperationException("Unable to read system memory from " + MEMINFO_FILE_PATH);
+        }
+
         return new OsMemoryStatusSnapshot(meminfo.getTotal(), meminfo.getAvailable());
     }
 

subprojects/process-services/src/test/groovy/org/gradle/process/internal/health/memory/CGroupMemoryInfoTest.groovy

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2023 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.gradle.process.internal.health.memory
+
+import spock.lang.Specification
+
+class CGroupMemoryInfoTest extends Specification {
+    private static final long MB_IN_BYTES = 1024 * 1024 * 1024
+
+    def "parses memory from cgroup values"() {
+        def snapshot = new CGroupMemoryInfo().getOsSnapshotFromCgroup(mbsToBytesAsString(800), mbsToBytesAsString(1024))
+
+        expect:
+        snapshot.totalPhysicalMemory == mbsToBytes(1024)
+        snapshot.freePhysicalMemory == mbsToBytes(224)
+    }
+
+    def "negative free memory returns zero"() {
+        def snapshot = new CGroupMemoryInfo().getOsSnapshotFromCgroup(mbsToBytesAsString(1024), mbsToBytesAsString(512))
+
+        expect:
+        snapshot.totalPhysicalMemory == mbsToBytes(512)
+        snapshot.freePhysicalMemory == 0
+    }
+
+    def "throws unsupported operation exception when non-numeric values are provided"() {
+        when:
+        new CGroupMemoryInfo().getOsSnapshotFromCgroup("foo", "bar")
+
+        then:
+        thrown(UnsupportedOperationException)
+    }
+
+    long mbsToBytes(int mbs) {
+        return mbs * MB_IN_BYTES
+    }
+
+    String mbsToBytesAsString(int mbs) {
+        return mbsToBytes(mbs) as String
+    }
+}