From 4ea3c4aa8118933fd22721dcf9b9e6c4a498736c Mon Sep 17 00:00:00 2001
From: Thierry Bordaz <tbordaz@redhat.com>
Date: Mon, 25 Nov 2019 10:59:44 +0100
Subject: [PATCH] Ticket 50736 - RetroCL trimming may crash at shutdown if
 trimming configuration is invalid

Bug Description:
	If config of retroCL trimming contains invalid value for trim-interval
        and/or maxage, then the trimming initialization is skipped.
        In such case the trimming structures are not allocated and if they
        are freed at shutdown it triggers a crash

Fix Description:
        When trimming mechanism is stopped (at shutdown) check that
        it was successfully initialized before freeing the structs

https://pagure.io/389-ds-base/issue/50736

Reviewed by: Mark Reynolds

Platforms tested: F30

Flag Day: no

Doc impact: no
---
 .../suites/replication/changelog_test.py      | 47 +++++++++++++++++++
 ldap/servers/plugins/retrocl/retrocl_trim.c   | 17 ++++---
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/dirsrvtests/tests/suites/replication/changelog_test.py b/dirsrvtests/tests/suites/replication/changelog_test.py
index a257e0272..e648478d0 100644
--- a/dirsrvtests/tests/suites/replication/changelog_test.py
+++ b/dirsrvtests/tests/suites/replication/changelog_test.py
@@ -16,6 +16,8 @@ from lib389.replica import Replicas
 from lib389.idm.user import UserAccounts
 from lib389.topologies import topology_m2 as topo
 from lib389._constants import *
+from lib389.plugins import RetroChangelogPlugin
+from lib389.dseldif import DSEldif
 from lib389.tasks import *
 from lib389.utils import *
 
@@ -452,6 +454,51 @@ def test_retrochangelog_maxage(topo, changelog_init):
 
     topo.ms["master1"].log.info("ticket47669 was successfully verified.")
 
+@pytest.mark.ds50736
+def test_retrochangelog_trimming_crash(topo, changelog_init):
+    """Check that when retroCL nsslapd-retrocthangelog contains invalid
+    value, then the instance does not crash at shutdown
+
+    :id: 5d9bd7ca-e9bf-4be9-8fc8-902aa5513052
+    :setup: Replication with two master, change nsslapd-changelogdir to
+    '/var/lib/dirsrv/slapd-master1/changelog' and
+    set cn=Retro Changelog Plugin,cn=plugins,cn=config to 'on'
+    :steps:
+        1. Set nsslapd-changelogmaxage in cn=Retro Changelog Plugin,cn=plugins,cn=config to value '-1'
+           This value is invalid. To disable retroCL trimming it should be set to 0
+        2. Do several restart
+        3. check there is no 'Detected Disorderly Shutdown' message (crash)
+        4. restore valid value for nsslapd-changelogmaxage '1w'
+
+    :expectedresults:
+        1. Operation should be successful
+        2. Operation should be successful
+        3. Operation should be successful
+        4. Operation should be successful
+     """
+    log.info('1. Test retroCL trimming crash in cn=Retro Changelog Plugin,cn=plugins,cn=config')
+
+    # set the nsslapd-changelogmaxage directly on dse.ldif
+    # because the set value is invalid
+    topo.ms["master1"].log.info("ticket50736 start verification")
+    topo.ms["master1"].stop()
+    retroPlugin = RetroChangelogPlugin(topo.ms["master1"])
+    dse_ldif = DSEldif(topo.ms["master1"])
+    dse_ldif.replace(retroPlugin.dn, 'nsslapd-changelogmaxage', '-1')
+    topo.ms["master1"].start()
+
+    # The crash should be systematic, but just in case do several restart
+    # with a delay to let all plugin init
+    for i in range(5):
+        time.sleep(1)
+        topo.ms["master1"].stop()
+        topo.ms["master1"].start()
+
+    assert not topo.ms["master1"].detectDisorderlyShutdown()
+
+    topo.ms["master1"].log.info("ticket 50736 was successfully verified.")
+
+
 
 if __name__ == '__main__':
     # Run isolated
diff --git a/ldap/servers/plugins/retrocl/retrocl_trim.c b/ldap/servers/plugins/retrocl/retrocl_trim.c
index a46534984..0378eb7f6 100644
--- a/ldap/servers/plugins/retrocl/retrocl_trim.c
+++ b/ldap/servers/plugins/retrocl/retrocl_trim.c
@@ -481,11 +481,16 @@ retrocl_init_trimming(void)
 void
 retrocl_stop_trimming(void)
 {
-    retrocl_trimming = 0;
-    if (retrocl_trim_ctx) {
-        slapi_eq_cancel(retrocl_trim_ctx);
-        retrocl_trim_ctx = NULL;
+    if (retrocl_trimming) {
+        /* RetroCL trimming config was valid and trimming struct allocated
+         * Let's free them
+         */
+        retrocl_trimming = 0;
+        if (retrocl_trim_ctx) {
+            slapi_eq_cancel(retrocl_trim_ctx);
+            retrocl_trim_ctx = NULL;
+        }
+        PR_DestroyLock(ts.ts_s_trim_mutex);
+        ts.ts_s_trim_mutex = NULL;
     }
-    PR_DestroyLock(ts.ts_s_trim_mutex);
-    ts.ts_s_trim_mutex = NULL;
 }
-- 
2.21.1