[389-commits] [389-ds-base] branch 389-ds-base-1.4.1 updated: Ticket 50736 - RetroCL trimming may crash at shutdown if trimming configuration is invalid

This is an automated email from the git hooks/post-receive script.

mreynolds pushed a commit to branch 389-ds-base-1.4.1
in repository 389-ds-base.

The following commit(s) were added to refs/heads/389-ds-base-1.4.1 by this push:
     new 7e39425  Ticket 50736 - RetroCL trimming may crash at shutdown if trimming
configuration is invalid
7e39425 is described below

commit 7e39425a9d71f62caeecc76da3186758b70a63cd
Author: Thierry Bordaz <tbordaz(a)redhat.com&gt;
AuthorDate: Mon Nov 25 10:59:44 2019 +0100

    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
---
 .../tests/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 a257e02..e648478 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.")
 
+(a)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 a465349..0378eb7 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;
 }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.