PR#50738: Ticket 50736 - RetroCL trimming may crash at shutdown if trimming configuration is invalid - 389-ds-base

389-ds-base

#50738 Ticket 50736 - RetroCL trimming may crash at shutdown if trimming configuration is invalid

Closed 3 years ago by spichugi. Opened 4 years ago by tbordaz.

tbordaz/389-ds-base ticket_50736 into master

Ticket 50736 - RetroCL trimming may crash at shutdown if trimming configuration is invalid

Thierry Bordaz • 4 years ago

94354d8

dirsrvtests/tests/suites/replication/changelog_test.py

file modified

+47

		`@@ -18,6 +18,8 @@`
		`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 *`

		`@@ -671,6 +673,51 @@`

		`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`

ldap/servers/plugins/retrocl/retrocl_trim.c

file modified

+11 -6

		`@@ -481,11 +481,16 @@`
		`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;`
		`}`

tbordaz commented 4 years ago

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: ?

Platforms tested: F30

Flag Day: no

Doc impact: no