661545
From 703cc4991049cdf3ad3506e432cda982b3b3b007 Mon Sep 17 00:00:00 2001
9ab0c5
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
9ab0c5
Date: Tue, 13 Feb 2018 14:37:11 +0100
9ab0c5
Subject: [PATCH] pid1: fix collection of cycles of units which reference one
9ab0c5
 another
9ab0c5
9ab0c5
A .socket will reference a .service unit, by registering a UnitRef with the
9ab0c5
.service unit. If this .service unit has the .socket unit listed in Wants or
9ab0c5
Sockets or such, a cycle will be created. We would not free this cycle
9ab0c5
properly, because we treated any unit with non-empty refs as uncollectable. To
9ab0c5
solve this issue, treats refs with UnitRef in u->refs_by_target similarly to
9ab0c5
the refs in u->dependencies, and check if the "other" unit is known to be
9ab0c5
needed. If it is not needed, do not treat the reference from it as preventing
9ab0c5
the unit we are looking at from being freed.
9ab0c5
9ab0c5
(cherry picked from commit 2641f02e23ac7d5385db7f932aff221a063f245e)
9ab0c5
661545
Resolves: #1718953
9ab0c5
---
9ab0c5
 src/core/manager.c | 14 ++++++++++++++
9ab0c5
 src/core/unit.c    |  9 +++++----
9ab0c5
 2 files changed, 19 insertions(+), 4 deletions(-)
9ab0c5
9ab0c5
diff --git a/src/core/manager.c b/src/core/manager.c
9ab0c5
index 9dfdd67860..fdbb3c0fd9 100644
9ab0c5
--- a/src/core/manager.c
9ab0c5
+++ b/src/core/manager.c
9ab0c5
@@ -888,6 +888,20 @@ static void unit_gc_sweep(Unit *u, unsigned gc_marker) {
9ab0c5
                         is_bad = false;
9ab0c5
         }
9ab0c5
 
9ab0c5
+        if (u->refs_by_target) {
9ab0c5
+                const UnitRef *ref;
9ab0c5
+
9ab0c5
+                LIST_FOREACH(refs_by_target, ref, u->refs_by_target) {
9ab0c5
+                        unit_gc_sweep(ref->source, gc_marker);
9ab0c5
+
9ab0c5
+                        if (ref->source->gc_marker == gc_marker + GC_OFFSET_GOOD)
9ab0c5
+                                goto good;
9ab0c5
+
9ab0c5
+                        if (ref->source->gc_marker != gc_marker + GC_OFFSET_BAD)
9ab0c5
+                                is_bad = false;
9ab0c5
+                }
9ab0c5
+        }
9ab0c5
+
9ab0c5
         if (is_bad)
9ab0c5
                 goto bad;
9ab0c5
 
9ab0c5
diff --git a/src/core/unit.c b/src/core/unit.c
9ab0c5
index 5376ef862f..2204be26d2 100644
9ab0c5
--- a/src/core/unit.c
9ab0c5
+++ b/src/core/unit.c
9ab0c5
@@ -288,7 +288,11 @@ bool unit_may_gc(Unit *u) {
9ab0c5
 
9ab0c5
         /* Checks whether the unit is ready to be unloaded for garbage collection.
9ab0c5
          * Returns true when the unit may be collected, and false if there's some
9ab0c5
-         * reason to keep it loaded. */
9ab0c5
+         * reason to keep it loaded.
9ab0c5
+         *
9ab0c5
+         * References from other units are *not* checked here. Instead, this is done
9ab0c5
+         * in unit_gc_sweep(), but using markers to properly collect dependency loops.
9ab0c5
+         */
9ab0c5
 
9ab0c5
         if (u->job)
9ab0c5
                 return false;
9ab0c5
@@ -315,9 +319,6 @@ bool unit_may_gc(Unit *u) {
9ab0c5
         if (u->no_gc)
9ab0c5
                 return false;
9ab0c5
 
9ab0c5
-        if (u->refs_by_target)
9ab0c5
-                return false;
9ab0c5
-
9ab0c5
         if (UNIT_VTABLE(u)->may_gc && !UNIT_VTABLE(u)->may_gc(u))
9ab0c5
                 return false;
9ab0c5