[389-ds-base] branch 389-ds-base-1.4.1 updated: Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
lkrispen 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 2a62015 Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
2a62015 is described below
commit 2a62015c3300c78ce9648f87f4008e26bfc72ba0
Author: Ludwig Krispenz <lkrispen(a)redhat.com>
AuthorDate: Thu Jan 30 17:26:35 2020 +0100
Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
Bug: If there are deadlocks a transaction will be retried. In the case
of modrdn operation there is an error in handling the newsuperior
dn, which has to be reset when the txn is repeated.
There is also an error in freeing the entry stored in the pblock which can
lead to a double free
There is also a memory leak for ec entries
Fix: check if the newsuperior in the pblock was changed before the retry and
only then free and reset it.
check and protect pblock entry from double free
remove ec entry from cache
There is also a message at shutdown that entries remain in the entry cache
although no leaks are reported and a hash dump didn't show entries.
Change log level to avoid confusion
Reviewed by: Thierry, William, Viktor - Thanks
---
ldap/servers/slapd/back-ldbm/cache.c | 2 +-
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 60 +++++++++++++++++++++---------
2 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c
index a03cdaa..89f958a 100644
--- a/ldap/servers/slapd/back-ldbm/cache.c
+++ b/ldap/servers/slapd/back-ldbm/cache.c
@@ -723,7 +723,7 @@ entrycache_clear_int(struct cache *cache)
}
cache->c_maxsize = size;
if (cache->c_curentries > 0) {
- slapi_log_err(SLAPI_LOG_WARNING,
+ slapi_log_err(SLAPI_LOG_CACHE,
"entrycache_clear_int", "There are still %" PRIu64 " entries "
"in the entry cache.\n",
cache->c_curentries);
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index 433ed88..2669801 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -67,6 +67,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
Slapi_DN *dn_newsuperiordn = NULL;
Slapi_DN dn_parentdn;
Slapi_DN *orig_dn_newsuperiordn = NULL;
+ Slapi_DN *pb_dn_newsuperiordn = NULL; /* used to check what is currently in the pblock */
Slapi_Entry *target_entry = NULL;
Slapi_Entry *original_targetentry = NULL;
int rc;
@@ -248,30 +249,45 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
slapi_sdn_set_dn_byref(&dn_newrdn, original_newrdn);
original_newrdn = slapi_ch_strdup(original_newrdn);
- slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &dn_newsuperiordn);
- slapi_sdn_free(&dn_newsuperiordn);
- slapi_pblock_set(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, orig_dn_newsuperiordn);
- dn_newsuperiordn = slapi_sdn_dup(orig_dn_newsuperiordn);
+ /* we need to restart with the original newsuperiordn which could have
+ * been modified. So check what is in the pblock, if it was changed
+ * free it, reset orig dn in th epblock and recreate a working superior
+ */
+ slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &pb_dn_newsuperiordn);
+ if (pb_dn_newsuperiordn != orig_dn_newsuperiordn) {
+ slapi_sdn_free(&pb_dn_newsuperiordn);
+ slapi_pblock_set(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, orig_dn_newsuperiordn);
+ dn_newsuperiordn = slapi_sdn_dup(orig_dn_newsuperiordn);
+ }
/* must duplicate ec before returning it to cache,
* which could free the entry. */
- if ((tmpentry = backentry_dup(original_entry ? original_entry : ec)) == NULL) {
+ if (!original_entry) {
+ slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modrdn",
+ "retrying transaction, but no original entry found\n");
+ ldap_result_code = LDAP_OPERATIONS_ERROR;
+ goto error_return;
+ }
+ if ((tmpentry = backentry_dup(original_entry)) == NULL) {
ldap_result_code = LDAP_OPERATIONS_ERROR;
goto error_return;
}
slapi_pblock_get(pb, SLAPI_MODRDN_EXISTING_ENTRY, &ent);
if (cache_is_in_cache(&inst->inst_cache, ec)) {
CACHE_REMOVE(&inst->inst_cache, ec);
- if (ent && (ent == ec->ep_entry)) {
- /*
- * On a retry, it's possible that ec is now stored in the
- * pblock as SLAPI_MODRDN_EXISTING_ENTRY. "ec" will be freed
- * by CACHE_RETURN below, so set ent to NULL so don't free
- * it again.
- */
- ent = NULL;
- }
+ }
+ if (ent && (ent == ec->ep_entry)) {
+ /*
+ * On a retry, it's possible that ec is now stored in the
+ * pblock as SLAPI_MODRDN_EXISTING_ENTRY. "ec" will be freed
+ * by CACHE_RETURN below, so set ent to NULL so don't free
+ * it again.
+ * And it needs to be checked always.
+ */
+ ent = NULL;
}
CACHE_RETURN(&inst->inst_cache, &ec);
+
+ /* LK why do we need this ????? */
if (!cache_is_in_cache(&inst->inst_cache, e)) {
if (CACHE_ADD(&inst->inst_cache, e, NULL) < 0) {
slapi_log_err(SLAPI_LOG_CACHE,
@@ -1087,8 +1103,9 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
if (slapi_sdn_get_dn(dn_newsuperiordn) != NULL) {
retval = ldbm_ancestorid_move_subtree(be, sdn, &dn_newdn, e->ep_id, children, &txn);
if (retval != 0) {
- if (retval == DB_LOCK_DEADLOCK)
+ if (retval == DB_LOCK_DEADLOCK) {
continue;
+ }
if (retval == DB_RUNRECOVERY || LDBM_OS_ERR_IS_DISKFULL(retval))
disk_full = 1;
MOD_SET_ERROR(ldap_result_code,
@@ -1108,8 +1125,9 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
e->ep_id, &txn, is_tombstone);
slapi_rdn_done(&newsrdn);
if (retval != 0) {
- if (retval == DB_LOCK_DEADLOCK)
+ if (retval == DB_LOCK_DEADLOCK) {
continue;
+ }
if (retval == DB_RUNRECOVERY || LDBM_OS_ERR_IS_DISKFULL(retval))
disk_full = 1;
MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);
@@ -1500,7 +1518,12 @@ common_return:
done_with_pblock_entry(pb, SLAPI_MODRDN_NEWPARENT_ENTRY);
done_with_pblock_entry(pb, SLAPI_MODRDN_TARGET_ENTRY);
slapi_ch_free_string(&original_newrdn);
- slapi_sdn_free(&orig_dn_newsuperiordn);
+ slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &pb_dn_newsuperiordn);
+ if (pb_dn_newsuperiordn != orig_dn_newsuperiordn) {
+ slapi_sdn_free(&orig_dn_newsuperiordn);
+ } else {
+ slapi_sdn_free(&dn_newsuperiordn);
+ }
backentry_free(&original_entry);
backentry_free(&tmpentry);
slapi_entry_free(original_targetentry);
@@ -1561,6 +1584,9 @@ moddn_unlock_and_return_entry(
/* Something bad happened so we should give back all the entries */
if (*targetentry != NULL) {
cache_unlock_entry(&inst->inst_cache, *targetentry);
+ if (cache_is_in_cache(&inst->inst_cache, *targetentry)) {
+ CACHE_REMOVE(&inst->inst_cache, *targetentry);
+ }
CACHE_RETURN(&inst->inst_cache, targetentry);
*targetentry = NULL;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.3.10 updated: Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
lkrispen pushed a commit to branch 389-ds-base-1.3.10
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.3.10 by this push:
new 7abd73c Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
7abd73c is described below
commit 7abd73c62cc04c38977c119b0d3254ec9e0d496f
Author: Ludwig Krispenz <lkrispen(a)redhat.com>
AuthorDate: Thu Jan 30 17:26:35 2020 +0100
Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
Bug: If there are deadlocks a transaction will be retried. In the case
of modrdn operation there is an error in handling the newsuperior
dn, which has to be reset when the txn is repeated.
There is also an error in freeing the entry stored in the pblock which can
lead to a double free
There is also a memory leak for ec entries
Fix: check if the newsuperior in the pblock was changed before the retry and
only then free and reset it.
check and protect pblock entry from double free
remove ec entry from cache
There is also a message at shutdown that entries remain in the entry cache
although no leaks are reported and a hash dump didn't show entries.
Change log level to avoid confusion
Reviewed by: Thierry, William, Viktor - Thanks
---
ldap/servers/slapd/back-ldbm/cache.c | 2 +-
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 60 +++++++++++++++++++++---------
2 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c
index 02453ab..c8d9f60 100644
--- a/ldap/servers/slapd/back-ldbm/cache.c
+++ b/ldap/servers/slapd/back-ldbm/cache.c
@@ -723,7 +723,7 @@ entrycache_clear_int(struct cache *cache)
}
cache->c_maxsize = size;
if (cache->c_curentries > 0) {
- slapi_log_err(SLAPI_LOG_WARNING,
+ slapi_log_err(SLAPI_LOG_CACHE,
"entrycache_clear_int", "There are still %" PRIu64 " entries "
"in the entry cache.\n",
cache->c_curentries);
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index 433ed88..2669801 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -67,6 +67,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
Slapi_DN *dn_newsuperiordn = NULL;
Slapi_DN dn_parentdn;
Slapi_DN *orig_dn_newsuperiordn = NULL;
+ Slapi_DN *pb_dn_newsuperiordn = NULL; /* used to check what is currently in the pblock */
Slapi_Entry *target_entry = NULL;
Slapi_Entry *original_targetentry = NULL;
int rc;
@@ -248,30 +249,45 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
slapi_sdn_set_dn_byref(&dn_newrdn, original_newrdn);
original_newrdn = slapi_ch_strdup(original_newrdn);
- slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &dn_newsuperiordn);
- slapi_sdn_free(&dn_newsuperiordn);
- slapi_pblock_set(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, orig_dn_newsuperiordn);
- dn_newsuperiordn = slapi_sdn_dup(orig_dn_newsuperiordn);
+ /* we need to restart with the original newsuperiordn which could have
+ * been modified. So check what is in the pblock, if it was changed
+ * free it, reset orig dn in th epblock and recreate a working superior
+ */
+ slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &pb_dn_newsuperiordn);
+ if (pb_dn_newsuperiordn != orig_dn_newsuperiordn) {
+ slapi_sdn_free(&pb_dn_newsuperiordn);
+ slapi_pblock_set(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, orig_dn_newsuperiordn);
+ dn_newsuperiordn = slapi_sdn_dup(orig_dn_newsuperiordn);
+ }
/* must duplicate ec before returning it to cache,
* which could free the entry. */
- if ((tmpentry = backentry_dup(original_entry ? original_entry : ec)) == NULL) {
+ if (!original_entry) {
+ slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modrdn",
+ "retrying transaction, but no original entry found\n");
+ ldap_result_code = LDAP_OPERATIONS_ERROR;
+ goto error_return;
+ }
+ if ((tmpentry = backentry_dup(original_entry)) == NULL) {
ldap_result_code = LDAP_OPERATIONS_ERROR;
goto error_return;
}
slapi_pblock_get(pb, SLAPI_MODRDN_EXISTING_ENTRY, &ent);
if (cache_is_in_cache(&inst->inst_cache, ec)) {
CACHE_REMOVE(&inst->inst_cache, ec);
- if (ent && (ent == ec->ep_entry)) {
- /*
- * On a retry, it's possible that ec is now stored in the
- * pblock as SLAPI_MODRDN_EXISTING_ENTRY. "ec" will be freed
- * by CACHE_RETURN below, so set ent to NULL so don't free
- * it again.
- */
- ent = NULL;
- }
+ }
+ if (ent && (ent == ec->ep_entry)) {
+ /*
+ * On a retry, it's possible that ec is now stored in the
+ * pblock as SLAPI_MODRDN_EXISTING_ENTRY. "ec" will be freed
+ * by CACHE_RETURN below, so set ent to NULL so don't free
+ * it again.
+ * And it needs to be checked always.
+ */
+ ent = NULL;
}
CACHE_RETURN(&inst->inst_cache, &ec);
+
+ /* LK why do we need this ????? */
if (!cache_is_in_cache(&inst->inst_cache, e)) {
if (CACHE_ADD(&inst->inst_cache, e, NULL) < 0) {
slapi_log_err(SLAPI_LOG_CACHE,
@@ -1087,8 +1103,9 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
if (slapi_sdn_get_dn(dn_newsuperiordn) != NULL) {
retval = ldbm_ancestorid_move_subtree(be, sdn, &dn_newdn, e->ep_id, children, &txn);
if (retval != 0) {
- if (retval == DB_LOCK_DEADLOCK)
+ if (retval == DB_LOCK_DEADLOCK) {
continue;
+ }
if (retval == DB_RUNRECOVERY || LDBM_OS_ERR_IS_DISKFULL(retval))
disk_full = 1;
MOD_SET_ERROR(ldap_result_code,
@@ -1108,8 +1125,9 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
e->ep_id, &txn, is_tombstone);
slapi_rdn_done(&newsrdn);
if (retval != 0) {
- if (retval == DB_LOCK_DEADLOCK)
+ if (retval == DB_LOCK_DEADLOCK) {
continue;
+ }
if (retval == DB_RUNRECOVERY || LDBM_OS_ERR_IS_DISKFULL(retval))
disk_full = 1;
MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);
@@ -1500,7 +1518,12 @@ common_return:
done_with_pblock_entry(pb, SLAPI_MODRDN_NEWPARENT_ENTRY);
done_with_pblock_entry(pb, SLAPI_MODRDN_TARGET_ENTRY);
slapi_ch_free_string(&original_newrdn);
- slapi_sdn_free(&orig_dn_newsuperiordn);
+ slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &pb_dn_newsuperiordn);
+ if (pb_dn_newsuperiordn != orig_dn_newsuperiordn) {
+ slapi_sdn_free(&orig_dn_newsuperiordn);
+ } else {
+ slapi_sdn_free(&dn_newsuperiordn);
+ }
backentry_free(&original_entry);
backentry_free(&tmpentry);
slapi_entry_free(original_targetentry);
@@ -1561,6 +1584,9 @@ moddn_unlock_and_return_entry(
/* Something bad happened so we should give back all the entries */
if (*targetentry != NULL) {
cache_unlock_entry(&inst->inst_cache, *targetentry);
+ if (cache_is_in_cache(&inst->inst_cache, *targetentry)) {
+ CACHE_REMOVE(&inst->inst_cache, *targetentry);
+ }
CACHE_RETURN(&inst->inst_cache, targetentry);
*targetentry = NULL;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.1 updated: Issue 50823 - dsctl doesn't work with 'slapd-' in the instance name
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
mhonek 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 9168d3c Issue 50823 - dsctl doesn't work with 'slapd-' in the instance name
9168d3c is described below
commit 9168d3cc08ecc5744c2d6b4ab2bb40869e349668
Author: Matus Honek <mhonek(a)redhat.com>
AuthorDate: Wed Jan 29 14:06:04 2020 +0000
Issue 50823 - dsctl doesn't work with 'slapd-' in the instance name
Bug Description:
DirSrv.list drops all occurrences of 'slapd-' within a serverid
rendering names containing it damaged.
Fix Description:
Remove only the first occurrence of 'slapd-' in the serverid, which is
the prefix that is expected to be removed.
Fixes https://pagure.io/389-ds-base/issue/50823
Author: Matus Honek <mhonek(a)redhat.com>
Review by: Mark, William (thanks!)
(cherry picked from commit 52930da0bb8abe94a56ff6dca5ea57347d3461a9)
---
src/lib389/lib389/__init__.py | 2 +-
src/lib389/lib389/instance/setup.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lib389/lib389/__init__.py b/src/lib389/lib389/__init__.py
index 0ff4335..3b39067 100644
--- a/src/lib389/lib389/__init__.py
+++ b/src/lib389/lib389/__init__.py
@@ -706,7 +706,7 @@ class DirSrv(SimpleLDAPObject, object):
if serverid is None and hasattr(self, 'serverid'):
serverid = self.serverid
elif serverid is not None:
- serverid = serverid.replace('slapd-', '')
+ serverid = serverid.replace('slapd-', '', 1)
if self.serverid is None:
# Need to set the Paths in case it does exist
diff --git a/src/lib389/lib389/instance/setup.py b/src/lib389/lib389/instance/setup.py
index 1bedd11..28e75e4 100644
--- a/src/lib389/lib389/instance/setup.py
+++ b/src/lib389/lib389/instance/setup.py
@@ -219,7 +219,7 @@ class SetupDs(object):
insts = inst.list(serverid=serverid)
if len(insts) != 1:
- log.error("No such instance to remove {}".format(serverid))
+ self.log.error("No such instance to remove {}".format(serverid))
return
inst.allocate(insts[0])
remove_ds_instance(inst, force=True)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.2 updated: Issue 50823 - dsctl doesn't work with 'slapd-' in the instance name
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
mhonek pushed a commit to branch 389-ds-base-1.4.2
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.2 by this push:
new 8d6f8f5 Issue 50823 - dsctl doesn't work with 'slapd-' in the instance name
8d6f8f5 is described below
commit 8d6f8f590e33730c4708919e26402c1c6902df03
Author: Matus Honek <mhonek(a)redhat.com>
AuthorDate: Wed Jan 29 14:06:04 2020 +0000
Issue 50823 - dsctl doesn't work with 'slapd-' in the instance name
Bug Description:
DirSrv.list drops all occurrences of 'slapd-' within a serverid
rendering names containing it damaged.
Fix Description:
Remove only the first occurrence of 'slapd-' in the serverid, which is
the prefix that is expected to be removed.
Fixes https://pagure.io/389-ds-base/issue/50823
Author: Matus Honek <mhonek(a)redhat.com>
Review by: Mark, William (thanks!)
(cherry picked from commit 52930da0bb8abe94a56ff6dca5ea57347d3461a9)
---
src/lib389/lib389/__init__.py | 2 +-
src/lib389/lib389/instance/setup.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lib389/lib389/__init__.py b/src/lib389/lib389/__init__.py
index 0d1ab57..003b686 100644
--- a/src/lib389/lib389/__init__.py
+++ b/src/lib389/lib389/__init__.py
@@ -711,7 +711,7 @@ class DirSrv(SimpleLDAPObject, object):
if serverid is None and hasattr(self, 'serverid'):
serverid = self.serverid
elif serverid is not None:
- serverid = serverid.replace('slapd-', '')
+ serverid = serverid.replace('slapd-', '', 1)
if self.serverid is None:
# Need to set the Paths in case it does exist
diff --git a/src/lib389/lib389/instance/setup.py b/src/lib389/lib389/instance/setup.py
index 1bedd11..28e75e4 100644
--- a/src/lib389/lib389/instance/setup.py
+++ b/src/lib389/lib389/instance/setup.py
@@ -219,7 +219,7 @@ class SetupDs(object):
insts = inst.list(serverid=serverid)
if len(insts) != 1:
- log.error("No such instance to remove {}".format(serverid))
+ self.log.error("No such instance to remove {}".format(serverid))
return
inst.allocate(insts[0])
remove_ds_instance(inst, force=True)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.2 updated: Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
lkrispen pushed a commit to branch 389-ds-base-1.4.2
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.2 by this push:
new 675a605 Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
675a605 is described below
commit 675a60500ba39edf361b1881c00b25b7613cad5c
Author: Ludwig Krispenz <lkrispen(a)redhat.com>
AuthorDate: Wed Jan 15 13:40:36 2020 +0100
Ticket 49624 cont - DB Deadlock on modrdn appears to corrupt database and entry cache
Bug: If there are deadlocks a transaction will be retried. In the case
of modrdn operation there is an error in handling the newsuperior
dn, which has to be reset when the txn is repeated.
There is also an error in freeing the entry stored in the pblock which can
lead to a double free
There is also a memory leak for ec entries
Fix: check if the newsuperior in the pblock was changed before the retry and
only then free and reset it.
check and protect pblock entry from double free
remove ec entry from cache
fix the txn_test_thread to run
There is also a message at shutdown that entries remain in the entry cache
although no leaks are reported and a hash dump didn't show entries.
Change log level to avoid confusion
Reviewed by: Thierry, William, Viktor - Thanks
---
ldap/servers/slapd/back-ldbm/cache.c | 2 +-
ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c | 2 +-
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 60 ++++++++++++++++++-------
3 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c
index a03cdaa..89f958a 100644
--- a/ldap/servers/slapd/back-ldbm/cache.c
+++ b/ldap/servers/slapd/back-ldbm/cache.c
@@ -723,7 +723,7 @@ entrycache_clear_int(struct cache *cache)
}
cache->c_maxsize = size;
if (cache->c_curentries > 0) {
- slapi_log_err(SLAPI_LOG_WARNING,
+ slapi_log_err(SLAPI_LOG_CACHE,
"entrycache_clear_int", "There are still %" PRIu64 " entries "
"in the entry cache.\n",
cache->c_curentries);
diff --git a/ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c b/ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c
index 1678529..ac45770 100644
--- a/ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c
+++ b/ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c
@@ -3064,7 +3064,7 @@ txn_test_threadmain(void *param)
txn_test_init_cfg(&cfg);
- if(BDB_CONFIG(li)->bdb_enable_transactions) {
+ if(!BDB_CONFIG(li)->bdb_enable_transactions) {
goto end;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index 433ed88..2669801 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -67,6 +67,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
Slapi_DN *dn_newsuperiordn = NULL;
Slapi_DN dn_parentdn;
Slapi_DN *orig_dn_newsuperiordn = NULL;
+ Slapi_DN *pb_dn_newsuperiordn = NULL; /* used to check what is currently in the pblock */
Slapi_Entry *target_entry = NULL;
Slapi_Entry *original_targetentry = NULL;
int rc;
@@ -248,30 +249,45 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
slapi_sdn_set_dn_byref(&dn_newrdn, original_newrdn);
original_newrdn = slapi_ch_strdup(original_newrdn);
- slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &dn_newsuperiordn);
- slapi_sdn_free(&dn_newsuperiordn);
- slapi_pblock_set(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, orig_dn_newsuperiordn);
- dn_newsuperiordn = slapi_sdn_dup(orig_dn_newsuperiordn);
+ /* we need to restart with the original newsuperiordn which could have
+ * been modified. So check what is in the pblock, if it was changed
+ * free it, reset orig dn in th epblock and recreate a working superior
+ */
+ slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &pb_dn_newsuperiordn);
+ if (pb_dn_newsuperiordn != orig_dn_newsuperiordn) {
+ slapi_sdn_free(&pb_dn_newsuperiordn);
+ slapi_pblock_set(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, orig_dn_newsuperiordn);
+ dn_newsuperiordn = slapi_sdn_dup(orig_dn_newsuperiordn);
+ }
/* must duplicate ec before returning it to cache,
* which could free the entry. */
- if ((tmpentry = backentry_dup(original_entry ? original_entry : ec)) == NULL) {
+ if (!original_entry) {
+ slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modrdn",
+ "retrying transaction, but no original entry found\n");
+ ldap_result_code = LDAP_OPERATIONS_ERROR;
+ goto error_return;
+ }
+ if ((tmpentry = backentry_dup(original_entry)) == NULL) {
ldap_result_code = LDAP_OPERATIONS_ERROR;
goto error_return;
}
slapi_pblock_get(pb, SLAPI_MODRDN_EXISTING_ENTRY, &ent);
if (cache_is_in_cache(&inst->inst_cache, ec)) {
CACHE_REMOVE(&inst->inst_cache, ec);
- if (ent && (ent == ec->ep_entry)) {
- /*
- * On a retry, it's possible that ec is now stored in the
- * pblock as SLAPI_MODRDN_EXISTING_ENTRY. "ec" will be freed
- * by CACHE_RETURN below, so set ent to NULL so don't free
- * it again.
- */
- ent = NULL;
- }
+ }
+ if (ent && (ent == ec->ep_entry)) {
+ /*
+ * On a retry, it's possible that ec is now stored in the
+ * pblock as SLAPI_MODRDN_EXISTING_ENTRY. "ec" will be freed
+ * by CACHE_RETURN below, so set ent to NULL so don't free
+ * it again.
+ * And it needs to be checked always.
+ */
+ ent = NULL;
}
CACHE_RETURN(&inst->inst_cache, &ec);
+
+ /* LK why do we need this ????? */
if (!cache_is_in_cache(&inst->inst_cache, e)) {
if (CACHE_ADD(&inst->inst_cache, e, NULL) < 0) {
slapi_log_err(SLAPI_LOG_CACHE,
@@ -1087,8 +1103,9 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
if (slapi_sdn_get_dn(dn_newsuperiordn) != NULL) {
retval = ldbm_ancestorid_move_subtree(be, sdn, &dn_newdn, e->ep_id, children, &txn);
if (retval != 0) {
- if (retval == DB_LOCK_DEADLOCK)
+ if (retval == DB_LOCK_DEADLOCK) {
continue;
+ }
if (retval == DB_RUNRECOVERY || LDBM_OS_ERR_IS_DISKFULL(retval))
disk_full = 1;
MOD_SET_ERROR(ldap_result_code,
@@ -1108,8 +1125,9 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
e->ep_id, &txn, is_tombstone);
slapi_rdn_done(&newsrdn);
if (retval != 0) {
- if (retval == DB_LOCK_DEADLOCK)
+ if (retval == DB_LOCK_DEADLOCK) {
continue;
+ }
if (retval == DB_RUNRECOVERY || LDBM_OS_ERR_IS_DISKFULL(retval))
disk_full = 1;
MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);
@@ -1500,7 +1518,12 @@ common_return:
done_with_pblock_entry(pb, SLAPI_MODRDN_NEWPARENT_ENTRY);
done_with_pblock_entry(pb, SLAPI_MODRDN_TARGET_ENTRY);
slapi_ch_free_string(&original_newrdn);
- slapi_sdn_free(&orig_dn_newsuperiordn);
+ slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &pb_dn_newsuperiordn);
+ if (pb_dn_newsuperiordn != orig_dn_newsuperiordn) {
+ slapi_sdn_free(&orig_dn_newsuperiordn);
+ } else {
+ slapi_sdn_free(&dn_newsuperiordn);
+ }
backentry_free(&original_entry);
backentry_free(&tmpentry);
slapi_entry_free(original_targetentry);
@@ -1561,6 +1584,9 @@ moddn_unlock_and_return_entry(
/* Something bad happened so we should give back all the entries */
if (*targetentry != NULL) {
cache_unlock_entry(&inst->inst_cache, *targetentry);
+ if (cache_is_in_cache(&inst->inst_cache, *targetentry)) {
+ CACHE_REMOVE(&inst->inst_cache, *targetentry);
+ }
CACHE_RETURN(&inst->inst_cache, targetentry);
*targetentry = NULL;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] 03/03: Issue 50824 - dsctl remove fails with "name 'ensure_str' is not defined"
by pagure＠pagure.io
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.
commit 6a683f7c2277ea9e56c89b641118ba91aedd790a
Author: Matus Honek <mhonek(a)redhat.com>
AuthorDate: Thu Jan 16 12:21:45 2020 +0100
Issue 50824 - dsctl remove fails with "name 'ensure_str' is not defined"
Bug Description:
Missing import since commit c39c7bb.
Fix Description:
Add the import.
Fixes https://pagure.io/389-ds-base/issue/50824
Author: Matus Honek <mhonek(a)redhat.com>
Review by: Mark (thanks!)
(cherry picked from commit 4f9aafca9a9927812da5e37ce71d79d1fd23b25a)
---
src/lib389/lib389/instance/remove.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lib389/lib389/instance/remove.py b/src/lib389/lib389/instance/remove.py
index e54e64e..e48b294 100644
--- a/src/lib389/lib389/instance/remove.py
+++ b/src/lib389/lib389/instance/remove.py
@@ -11,7 +11,7 @@ import shutil
import subprocess
import logging
from lib389.nss_ssl import NssSsl
-from lib389.utils import selinux_label_port, assert_c, ensure_list_str
+from lib389.utils import selinux_label_port, assert_c, ensure_str, ensure_list_str
######################## WARNING #############################
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] 02/03: Issue 50798 - incorrect bytes in format string(fix import issue)
by pagure＠pagure.io
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.
commit 8da2aadab751a251c0d77dde74ba9b1ee57433af
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Mon Jan 13 17:17:40 2020 -0500
Issue 50798 - incorrect bytes in format string(fix import issue)
Description: The previous commit did not import ensure_list_str() from
utils.py
relates: https://pagure.io/389-ds-base/issue/50798
Reviewed by: mreynolds (one line commit rule)
---
src/lib389/lib389/instance/remove.py | 2 +-
src/lib389/lib389/instance/setup.py | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/lib389/lib389/instance/remove.py b/src/lib389/lib389/instance/remove.py
index 41eaeb4..e54e64e 100644
--- a/src/lib389/lib389/instance/remove.py
+++ b/src/lib389/lib389/instance/remove.py
@@ -11,7 +11,7 @@ import shutil
import subprocess
import logging
from lib389.nss_ssl import NssSsl
-from lib389.utils import selinux_label_port, assert_c
+from lib389.utils import selinux_label_port, assert_c, ensure_list_str
######################## WARNING #############################
diff --git a/src/lib389/lib389/instance/setup.py b/src/lib389/lib389/instance/setup.py
index ae814d1..1bedd11 100644
--- a/src/lib389/lib389/instance/setup.py
+++ b/src/lib389/lib389/instance/setup.py
@@ -33,6 +33,7 @@ from lib389.utils import (
assert_c,
is_a_dn,
ensure_str,
+ ensure_list_str,
normalizeDN,
socket_check_open,
selinux_label_port,
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] 01/03: Ticket 50798 - incorrect bytes in format string
by pagure＠pagure.io
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.
commit 3dad1f3621a0bc858db544f93656b8441d7aac82
Author: William Brown <william(a)blackhats.net.au>
AuthorDate: Mon Dec 30 14:18:16 2019 +1100
Ticket 50798 - incorrect bytes in format string
Bug Description: We did not use ensure_bytes on a command output in
format strings. Python 3 subprocess returens bytes, but format string
expects utf8
Fix Description: Wrap the values in the correct safety wrappers.
https://pagure.io/389-ds-base/issue/50798
Author: William Brown <william(a)blackhats.net.au>
Review by: mreynolds, mhonek (Thanks)
---
src/lib389/lib389/instance/remove.py | 5 ++++-
src/lib389/lib389/instance/setup.py | 5 ++++-
src/lib389/lib389/utils.py | 5 ++++-
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/lib389/lib389/instance/remove.py b/src/lib389/lib389/instance/remove.py
index c9a872e..41eaeb4 100644
--- a/src/lib389/lib389/instance/remove.py
+++ b/src/lib389/lib389/instance/remove.py
@@ -102,7 +102,10 @@ def remove_ds_instance(dirsrv, force=False):
result = subprocess.run(["systemctl", "disable", "dirsrv@{}".format(dirsrv.serverid)],
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
- _log.debug(f"CMD: {' '.join(result.args)} ; STDOUT: {result.stdout} ; STDERR: {result.stderr}")
+ args = ' '.join(ensure_list_str(result.args))
+ stdout = ensure_str(result.stdout)
+ stderr = ensure_str(result.stderr)
+ _log.debug(f"CMD: {args} ; STDOUT: {stdout} ; STDERR: {stderr}")
_log.debug("Removing %s" % tmpfiles_d_path)
try:
diff --git a/src/lib389/lib389/instance/setup.py b/src/lib389/lib389/instance/setup.py
index 073c7c7..ae814d1 100644
--- a/src/lib389/lib389/instance/setup.py
+++ b/src/lib389/lib389/instance/setup.py
@@ -783,7 +783,10 @@ class SetupDs(object):
# Should create the symlink we need, but without starting it.
result = subprocess.run(["systemctl", "enable", "dirsrv@%s" % slapd['instance_name']],
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
- self.log.debug(f"CMD: {' '.join(result.args)} ; STDOUT: {result.stdout} ; STDERR: {result.stderr}")
+ args = ' '.join(ensure_list_str(result.args))
+ stdout = ensure_str(result.stdout)
+ stderr = ensure_str(result.stderr)
+ self.log.debug(f"CMD: {args} ; STDOUT: {stdout} ; STDERR: {stderr}")
# Setup tmpfiles_d
tmpfile_d = ds_paths.tmpfiles_d + "/dirsrv-" + slapd['instance_name'] + ".conf"
diff --git a/src/lib389/lib389/utils.py b/src/lib389/lib389/utils.py
index 292d1a7..a2c1b6a 100644
--- a/src/lib389/lib389/utils.py
+++ b/src/lib389/lib389/utils.py
@@ -304,7 +304,10 @@ def selinux_label_port(port, remove_label=False):
"-p", "tcp", str(port)],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
- log.debug(f"CMD: {' '.join(result.args)} ; STDOUT: {result.stdout} ; STDERR: {result.stderr}")
+ args = ' '.join(ensure_list_str(result.args))
+ stdout = ensure_str(result.stdout)
+ stderr = ensure_str(result.stderr)
+ log.debug(f"CMD: {args} ; STDOUT: {stdout} ; STDERR: {stderr}")
return
except (OSError, subprocess.CalledProcessError) as e:
label_ex = e
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.3.10 updated: Ticket 50709: (cont) Several memory leaks reported by Valgrind for 389-ds 1.3.9.1-10
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
tbordaz pushed a commit to branch 389-ds-base-1.3.10
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.3.10 by this push:
new dab015b Ticket 50709: (cont) Several memory leaks reported by Valgrind for 389-ds 1.3.9.1-10
dab015b is described below
commit dab015b9504b3d452504156f31762983c17f92f0
Author: Thierry Bordaz <tbordaz(a)redhat.com>
AuthorDate: Fri Jan 24 10:56:23 2020 +0100
Ticket 50709: (cont) Several memory leaks reported by Valgrind for 389-ds 1.3.9.1-10
Bug Description:
Cherry pick from master was broken because connection locking
uses pthread lock in master while it remains Monitor in 1.3.10
Fix Description:
Change the locking function to use Monitor in that branch
https://pagure.io/389-ds-base/issue/50709
Reviewed by: thierry bordaz
Platforms tested: 7.8
Flag Day: no
Doc impact: no
---
ldap/servers/slapd/pblock.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c
index d2ad614..d21cf7e 100644
--- a/ldap/servers/slapd/pblock.c
+++ b/ldap/servers/slapd/pblock.c
@@ -486,9 +486,9 @@ slapi_pblock_get(Slapi_PBlock *pblock, int arg, void *value)
if (pblock->pb_conn == NULL) {
break;
}
- pthread_mutex_lock(&(pblock->pb_conn->c_mutex));
+ PR_EnterMonitor(pblock->pb_conn->c_mutex);
(*(PRNetAddr **) value) = pblock->pb_conn->cin_addr_aclip;
- pthread_mutex_unlock(&(pblock->pb_conn->c_mutex));
+ PR_ExitMonitor(pblock->pb_conn->c_mutex);
break;
case SLAPI_CONN_SERVERNETADDR:
if (pblock->pb_conn == NULL) {
@@ -2583,10 +2583,10 @@ slapi_pblock_set(Slapi_PBlock *pblock, int arg, void *value)
if (pblock->pb_conn == NULL) {
break;
}
- pthread_mutex_lock(&(pblock->pb_conn->c_mutex));
+ PR_EnterMonitor(pblock->pb_conn->c_mutex);
slapi_ch_free((void **)&pblock->pb_conn->cin_addr_aclip);
pblock->pb_conn->cin_addr_aclip = (PRNetAddr *)value;
- pthread_mutex_unlock(&(pblock->pb_conn->c_mutex));
+ PR_ExitMonitor(pblock->pb_conn->c_mutex);
case SLAPI_CONN_IS_REPLICATION_SESSION:
if (pblock->pb_conn == NULL) {
slapi_log_err(SLAPI_LOG_ERR,
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.1 updated: Issue 50850 - Fix dsctl healthcheck for python36
by pagure＠pagure.io
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 3d5bb2c Issue 50850 - Fix dsctl healthcheck for python36
3d5bb2c is described below
commit 3d5bb2c2636e77f8655fd4c599526f0303fc3db1
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Thu Jan 23 12:22:21 2020 -0500
Issue 50850 - Fix dsctl healthcheck for python36
Description: dsctl health check, specifically the certificate expiring
checks, were using python37 specific functions, but these
do not work on python36. Needed to replace fromisoformat()
with something more portable.
relates: https://pagure.io/389-ds-base/issue/50850
Reviewed by: firstyear(Thanks!)
---
src/lib389/lib389/nss_ssl.py | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/lib389/lib389/nss_ssl.py b/src/lib389/lib389/nss_ssl.py
index 587adcd..77ddfe2 100644
--- a/src/lib389/lib389/nss_ssl.py
+++ b/src/lib389/lib389/nss_ssl.py
@@ -79,13 +79,15 @@ class NssSsl(object):
cert_list.append(self.get_cert_details(cert[0]))
for cert in cert_list:
- if date.fromisoformat(cert[3].split()[0]) - date.today() < timedelta(days=0):
+ cert_date = cert[3].split()[0]
+ diff_date = datetime.strptime(cert_date, '%Y-%m-%d').date() - datetime.today().date()
+ if diff_date < timedelta(days=0):
# Expired
report = copy.deepcopy(DSCERTLE0002)
report['detail'] = report['detail'].replace('CERT', cert[0])
yield report
- elif date.fromisoformat(cert[3].split()[0]) - date.today() < timedelta(days=30):
- # Expiring
+ elif diff_date < timedelta(days=30):
+ # Expiring within 30 days
report = copy.deepcopy(DSCERTLE0001)
report['detail'] = report['detail'].replace('CERT', cert[0])
yield report
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months