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