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