|
 |
dc8c34 |
From 4d825079a9bb622211d3709ea2496f533b8e51d1 Mon Sep 17 00:00:00 2001
|
|
|
dc8c34 |
From: Noriko Hosoi <nhosoi@totoro.usersys.redhat.com>
|
|
|
dc8c34 |
Date: Fri, 5 Oct 2012 17:56:22 -0700
|
|
|
dc8c34 |
Subject: [PATCH 2/5] Bug 863576 - Dirsrv deadlock locking up IPA
|
|
|
dc8c34 |
|
|
|
dc8c34 |
https://bugzilla.redhat.com/show_bug.cgi?id=863576
|
|
|
dc8c34 |
|
|
|
dc8c34 |
Bug Description: Abandon of a Simple Paged Results request causes
|
|
|
dc8c34 |
the self deadlock. When abandoning a simple paged result request,
|
|
|
dc8c34 |
the mutex for the connection object c_mutex is locked in do_abandon.
|
|
|
dc8c34 |
But to free a pagedresult massage id, pagedresults_free_one_msgid
|
|
|
dc8c34 |
called from do_abandon tries to acquire lock on c_mutex again.
|
|
|
dc8c34 |
The NSPR lock function PR_Lock is not self re-entrant. Thus the
|
|
|
dc8c34 |
server hangs there due to the self-deadlock.
|
|
|
dc8c34 |
|
|
|
dc8c34 |
Fix Description: This patch is removing to call PR_Lock(c_mutex)
|
|
|
dc8c34 |
in pagedresults_free_one_msgid and renamed it to pagedresults_free_
|
|
|
dc8c34 |
one_msgid_nolock. To maintain the consistency, "_nolock" is added
|
|
|
dc8c34 |
to other pagedresults apis which do not call PR_Lock in it.
|
|
|
dc8c34 |
Also, stricter locking on c_mutex is being added to pagedresults_
|
|
|
dc8c34 |
parse_control_value to protect the pagedresults related field in
|
|
|
dc8c34 |
the connection object.
|
|
|
dc8c34 |
(cherry picked from commit c19bb9dd1e95ee98a53a06f3d7eefb4dce5bc0ef)
|
|
|
dc8c34 |
---
|
|
|
dc8c34 |
ldap/servers/slapd/abandon.c | 2 +-
|
|
|
dc8c34 |
ldap/servers/slapd/connection.c | 4 +-
|
|
|
dc8c34 |
ldap/servers/slapd/daemon.c | 2 +-
|
|
|
dc8c34 |
ldap/servers/slapd/pagedresults.c | 130 +++++++++++++++++++------------------
|
|
|
dc8c34 |
ldap/servers/slapd/proto-slap.h | 8 +-
|
|
|
dc8c34 |
5 files changed, 74 insertions(+), 72 deletions(-)
|
|
|
dc8c34 |
|
|
|
dc8c34 |
diff --git a/ldap/servers/slapd/abandon.c b/ldap/servers/slapd/abandon.c
|
|
|
dc8c34 |
index 4f00da9..094ae95 100644
|
|
|
dc8c34 |
--- a/ldap/servers/slapd/abandon.c
|
|
|
dc8c34 |
+++ b/ldap/servers/slapd/abandon.c
|
|
|
dc8c34 |
@@ -153,7 +153,7 @@ do_abandon( Slapi_PBlock *pb )
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
|
|
|
dc8c34 |
if ( op_is_pagedresults(o) ) {
|
|
|
dc8c34 |
- if ( 0 == pagedresults_free_one_msgid(pb->pb_conn, id) ) {
|
|
|
dc8c34 |
+ if ( 0 == pagedresults_free_one_msgid_nolock(pb->pb_conn, id) ) {
|
|
|
dc8c34 |
slapi_log_access( LDAP_DEBUG_STATS, "conn=%" NSPRIu64
|
|
|
dc8c34 |
" op=%d ABANDON targetop=Simple Paged Results\n",
|
|
|
dc8c34 |
pb->pb_conn->c_connid, pb->pb_op->o_opid );
|
|
|
dc8c34 |
diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
|
|
|
dc8c34 |
index 9e43104..a3b1df5 100644
|
|
|
dc8c34 |
--- a/ldap/servers/slapd/connection.c
|
|
|
dc8c34 |
+++ b/ldap/servers/slapd/connection.c
|
|
|
dc8c34 |
@@ -2094,7 +2094,7 @@ void connection_enter_leave_turbo(Connection *conn, int current_turbo_flag, int
|
|
|
dc8c34 |
PR_Lock(conn->c_mutex);
|
|
|
dc8c34 |
/* We can already be in turbo mode, or not */
|
|
|
dc8c34 |
current_mode = current_turbo_flag;
|
|
|
dc8c34 |
- if (pagedresults_in_use(conn)) {
|
|
|
dc8c34 |
+ if (pagedresults_in_use_nolock(conn)) {
|
|
|
dc8c34 |
/* PAGED_RESULTS does not need turbo mode */
|
|
|
dc8c34 |
new_mode = 0;
|
|
|
dc8c34 |
} else if (conn->c_private->operation_rate == 0) {
|
|
|
dc8c34 |
@@ -2780,7 +2780,7 @@ disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int opid, PRErro
|
|
|
dc8c34 |
connection_abandon_operations( conn );
|
|
|
dc8c34 |
/* needed here to ensure simple paged results timeout properly and
|
|
|
dc8c34 |
* don't impact subsequent ops */
|
|
|
dc8c34 |
- pagedresults_reset_timedout(conn);
|
|
|
dc8c34 |
+ pagedresults_reset_timedout_nolock(conn);
|
|
|
dc8c34 |
|
|
|
dc8c34 |
if (! config_check_referral_mode()) {
|
|
|
dc8c34 |
/*
|
|
|
dc8c34 |
diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
|
|
|
dc8c34 |
index 597e131..b611f5c 100644
|
|
|
dc8c34 |
--- a/ldap/servers/slapd/daemon.c
|
|
|
dc8c34 |
+++ b/ldap/servers/slapd/daemon.c
|
|
|
dc8c34 |
@@ -1693,7 +1693,7 @@ setup_pr_read_pds(Connection_Table *ct, PRFileDesc **n_tcps, PRFileDesc **s_tcps
|
|
|
dc8c34 |
{
|
|
|
dc8c34 |
int add_fd = 1;
|
|
|
dc8c34 |
/* check timeout for PAGED RESULTS */
|
|
|
dc8c34 |
- if (pagedresults_is_timedout(c))
|
|
|
dc8c34 |
+ if (pagedresults_is_timedout_nolock(c))
|
|
|
dc8c34 |
{
|
|
|
dc8c34 |
/* Exceeded the timelimit; disconnect the client */
|
|
|
dc8c34 |
disconnect_server_nomutex(c, c->c_connid, -1,
|
|
|
dc8c34 |
diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c
|
|
|
dc8c34 |
index ea7de14..d445c06 100644
|
|
|
dc8c34 |
--- a/ldap/servers/slapd/pagedresults.c
|
|
|
dc8c34 |
+++ b/ldap/servers/slapd/pagedresults.c
|
|
|
dc8c34 |
@@ -64,6 +64,7 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
|
|
|
dc8c34 |
struct berval cookie = {0};
|
|
|
dc8c34 |
Connection *conn = pb->pb_conn;
|
|
|
dc8c34 |
Operation *op = pb->pb_op;
|
|
|
dc8c34 |
+ BerElement *ber = NULL;
|
|
|
dc8c34 |
|
|
|
dc8c34 |
LDAPDebug0Args(LDAP_DEBUG_TRACE, "--> pagedresults_parse_control_value\n");
|
|
|
dc8c34 |
if ( NULL == conn || NULL == op || NULL == pagesize || NULL == index ) {
|
|
|
dc8c34 |
@@ -76,70 +77,71 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
|
|
|
dc8c34 |
|
|
|
dc8c34 |
if ( psbvp->bv_len == 0 || psbvp->bv_val == NULL )
|
|
|
dc8c34 |
{
|
|
|
dc8c34 |
- rc = LDAP_PROTOCOL_ERROR;
|
|
|
dc8c34 |
+ LDAPDebug0Args(LDAP_DEBUG_ANY,
|
|
|
dc8c34 |
+ "<-- pagedresults_parse_control_value: no control value\n");
|
|
|
dc8c34 |
+ return LDAP_PROTOCOL_ERROR;
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
- else
|
|
|
dc8c34 |
+ ber = ber_init( psbvp );
|
|
|
dc8c34 |
+ if ( ber == NULL )
|
|
|
dc8c34 |
{
|
|
|
dc8c34 |
- BerElement *ber = ber_init( psbvp );
|
|
|
dc8c34 |
- if ( ber == NULL )
|
|
|
dc8c34 |
- {
|
|
|
dc8c34 |
- rc = LDAP_OPERATIONS_ERROR;
|
|
|
dc8c34 |
- }
|
|
|
dc8c34 |
- else
|
|
|
dc8c34 |
- {
|
|
|
dc8c34 |
- if ( ber_scanf( ber, "{io}", pagesize, &cookie ) == LBER_ERROR )
|
|
|
dc8c34 |
- {
|
|
|
dc8c34 |
- rc = LDAP_PROTOCOL_ERROR;
|
|
|
dc8c34 |
+ LDAPDebug0Args(LDAP_DEBUG_ANY,
|
|
|
dc8c34 |
+ "<-- pagedresults_parse_control_value: no control value\n");
|
|
|
dc8c34 |
+ return LDAP_PROTOCOL_ERROR;
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
+ if ( ber_scanf( ber, "{io}", pagesize, &cookie ) == LBER_ERROR )
|
|
|
dc8c34 |
+ {
|
|
|
dc8c34 |
+ LDAPDebug0Args(LDAP_DEBUG_ANY,
|
|
|
dc8c34 |
+ "<-- pagedresults_parse_control_value: corrupted control value\n");
|
|
|
dc8c34 |
+ return LDAP_PROTOCOL_ERROR;
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
+
|
|
|
dc8c34 |
+ PR_Lock(conn->c_mutex);
|
|
|
dc8c34 |
+ /* the ber encoding is no longer needed */
|
|
|
dc8c34 |
+ ber_free(ber, 1);
|
|
|
dc8c34 |
+ if ( cookie.bv_len <= 0 ) {
|
|
|
dc8c34 |
+ int i;
|
|
|
dc8c34 |
+ int maxlen;
|
|
|
dc8c34 |
+ /* first time? */
|
|
|
dc8c34 |
+ maxlen = conn->c_pagedresults.prl_maxlen;
|
|
|
dc8c34 |
+ if (conn->c_pagedresults.prl_count == maxlen) {
|
|
|
dc8c34 |
+ if (0 == maxlen) { /* first time */
|
|
|
dc8c34 |
+ conn->c_pagedresults.prl_maxlen = 1;
|
|
|
dc8c34 |
+ conn->c_pagedresults.prl_list =
|
|
|
dc8c34 |
+ (PagedResults *)slapi_ch_calloc(1,
|
|
|
dc8c34 |
+ sizeof(PagedResults));
|
|
|
dc8c34 |
+ } else {
|
|
|
dc8c34 |
+ /* new max length */
|
|
|
dc8c34 |
+ conn->c_pagedresults.prl_maxlen *= 2;
|
|
|
dc8c34 |
+ conn->c_pagedresults.prl_list =
|
|
|
dc8c34 |
+ (PagedResults *)slapi_ch_realloc(
|
|
|
dc8c34 |
+ (char *)conn->c_pagedresults.prl_list,
|
|
|
dc8c34 |
+ sizeof(PagedResults) *
|
|
|
dc8c34 |
+ conn->c_pagedresults.prl_maxlen);
|
|
|
dc8c34 |
+ /* initialze newly allocated area */
|
|
|
dc8c34 |
+ memset(conn->c_pagedresults.prl_list + maxlen, '\0',
|
|
|
dc8c34 |
+ sizeof(PagedResults) * maxlen);
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
- /* the ber encoding is no longer needed */
|
|
|
dc8c34 |
- ber_free(ber, 1);
|
|
|
dc8c34 |
- if ( cookie.bv_len <= 0 ) {
|
|
|
dc8c34 |
- int i;
|
|
|
dc8c34 |
- int maxlen;
|
|
|
dc8c34 |
- /* first time? */
|
|
|
dc8c34 |
- PR_Lock(conn->c_mutex);
|
|
|
dc8c34 |
- maxlen = conn->c_pagedresults.prl_maxlen;
|
|
|
dc8c34 |
- if (conn->c_pagedresults.prl_count == maxlen) {
|
|
|
dc8c34 |
- if (0 == maxlen) { /* first time */
|
|
|
dc8c34 |
- conn->c_pagedresults.prl_maxlen = 1;
|
|
|
dc8c34 |
- conn->c_pagedresults.prl_list =
|
|
|
dc8c34 |
- (PagedResults *)slapi_ch_calloc(1,
|
|
|
dc8c34 |
- sizeof(PagedResults));
|
|
|
dc8c34 |
- } else {
|
|
|
dc8c34 |
- /* new max length */
|
|
|
dc8c34 |
- conn->c_pagedresults.prl_maxlen *= 2;
|
|
|
dc8c34 |
- conn->c_pagedresults.prl_list =
|
|
|
dc8c34 |
- (PagedResults *)slapi_ch_realloc(
|
|
|
dc8c34 |
- (char *)conn->c_pagedresults.prl_list,
|
|
|
dc8c34 |
- sizeof(PagedResults) *
|
|
|
dc8c34 |
- conn->c_pagedresults.prl_maxlen);
|
|
|
dc8c34 |
- /* initialze newly allocated area */
|
|
|
dc8c34 |
- memset(conn->c_pagedresults.prl_list + maxlen, '\0',
|
|
|
dc8c34 |
- sizeof(PagedResults) * maxlen);
|
|
|
dc8c34 |
- }
|
|
|
dc8c34 |
- *index = maxlen; /* the first position in the new area */
|
|
|
dc8c34 |
- } else {
|
|
|
dc8c34 |
- for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++) {
|
|
|
dc8c34 |
- if (!conn->c_pagedresults.prl_list[i].pr_current_be) {
|
|
|
dc8c34 |
- *index = i;
|
|
|
dc8c34 |
- break;
|
|
|
dc8c34 |
- }
|
|
|
dc8c34 |
- }
|
|
|
dc8c34 |
+ *index = maxlen; /* the first position in the new area */
|
|
|
dc8c34 |
+ } else {
|
|
|
dc8c34 |
+ for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++) {
|
|
|
dc8c34 |
+ if (!conn->c_pagedresults.prl_list[i].pr_current_be) {
|
|
|
dc8c34 |
+ *index = i;
|
|
|
dc8c34 |
+ break;
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
- conn->c_pagedresults.prl_count++;
|
|
|
dc8c34 |
- PR_Unlock(conn->c_mutex);
|
|
|
dc8c34 |
- } else {
|
|
|
dc8c34 |
- /* Repeated paged results request.
|
|
|
dc8c34 |
- * PagedResults is already allocated. */
|
|
|
dc8c34 |
- char *ptr = slapi_ch_malloc(cookie.bv_len + 1);
|
|
|
dc8c34 |
- memcpy(ptr, cookie.bv_val, cookie.bv_len);
|
|
|
dc8c34 |
- *(ptr+cookie.bv_len) = '\0';
|
|
|
dc8c34 |
- *index = strtol(ptr, NULL, 10);
|
|
|
dc8c34 |
- slapi_ch_free_string(&ptr);
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
- slapi_ch_free((void **)&cookie.bv_val);
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
+ conn->c_pagedresults.prl_count++;
|
|
|
dc8c34 |
+ } else {
|
|
|
dc8c34 |
+ /* Repeated paged results request.
|
|
|
dc8c34 |
+ * PagedResults is already allocated. */
|
|
|
dc8c34 |
+ char *ptr = slapi_ch_malloc(cookie.bv_len + 1);
|
|
|
dc8c34 |
+ memcpy(ptr, cookie.bv_val, cookie.bv_len);
|
|
|
dc8c34 |
+ *(ptr+cookie.bv_len) = '\0';
|
|
|
dc8c34 |
+ *index = strtol(ptr, NULL, 10);
|
|
|
dc8c34 |
+ slapi_ch_free_string(&ptr);
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
+ slapi_ch_free((void **)&cookie.bv_val);
|
|
|
dc8c34 |
+
|
|
|
dc8c34 |
if ((*index > -1) && (*index < conn->c_pagedresults.prl_maxlen)) {
|
|
|
dc8c34 |
/* Need to keep the latest msgid to prepare for the abandon. */
|
|
|
dc8c34 |
conn->c_pagedresults.prl_list[*index].pr_msgid = op->o_msgid;
|
|
|
dc8c34 |
@@ -149,6 +151,7 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
|
|
|
dc8c34 |
"pagedresults_parse_control_value: invalid cookie: %d\n",
|
|
|
dc8c34 |
*index);
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
+ PR_Unlock(conn->c_mutex);
|
|
|
dc8c34 |
|
|
|
dc8c34 |
LDAPDebug1Arg(LDAP_DEBUG_TRACE,
|
|
|
dc8c34 |
"<-- pagedresults_parse_control_value: idx %d\n", *index);
|
|
|
dc8c34 |
@@ -261,7 +264,7 @@ pagedresults_free_one( Connection *conn, int index )
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
|
|
|
dc8c34 |
int
|
|
|
dc8c34 |
-pagedresults_free_one_msgid( Connection *conn, ber_int_t msgid )
|
|
|
dc8c34 |
+pagedresults_free_one_msgid_nolock( Connection *conn, ber_int_t msgid )
|
|
|
dc8c34 |
{
|
|
|
dc8c34 |
int rc = -1;
|
|
|
dc8c34 |
int i;
|
|
|
dc8c34 |
@@ -269,9 +272,9 @@ pagedresults_free_one_msgid( Connection *conn, ber_int_t msgid )
|
|
|
dc8c34 |
LDAPDebug1Arg(LDAP_DEBUG_TRACE,
|
|
|
dc8c34 |
"--> pagedresults_free_one: msgid=%d\n", msgid);
|
|
|
dc8c34 |
if (conn && (msgid > -1)) {
|
|
|
dc8c34 |
- PR_Lock(conn->c_mutex);
|
|
|
dc8c34 |
if (conn->c_pagedresults.prl_count <= 0) {
|
|
|
dc8c34 |
- LDAPDebug2Args(LDAP_DEBUG_TRACE, "pagedresults_free_one_msgid: "
|
|
|
dc8c34 |
+ LDAPDebug2Args(LDAP_DEBUG_TRACE,
|
|
|
dc8c34 |
+ "pagedresults_free_one_msgid_nolock: "
|
|
|
dc8c34 |
"conn=%d paged requests list count is %d\n",
|
|
|
dc8c34 |
conn->c_connid, conn->c_pagedresults.prl_count);
|
|
|
dc8c34 |
} else {
|
|
|
dc8c34 |
@@ -285,7 +288,6 @@ pagedresults_free_one_msgid( Connection *conn, ber_int_t msgid )
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
- PR_Unlock(conn->c_mutex);
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
|
|
|
dc8c34 |
LDAPDebug1Arg(LDAP_DEBUG_TRACE, "<-- pagedresults_free_one: %d\n", rc);
|
|
|
dc8c34 |
@@ -720,7 +722,7 @@ pagedresults_reset_processing(Connection *conn, int index)
|
|
|
dc8c34 |
|
|
|
dc8c34 |
/* Are all the paged results requests timed out? */
|
|
|
dc8c34 |
int
|
|
|
dc8c34 |
-pagedresults_is_timedout(Connection *conn)
|
|
|
dc8c34 |
+pagedresults_is_timedout_nolock(Connection *conn)
|
|
|
dc8c34 |
{
|
|
|
dc8c34 |
int i;
|
|
|
dc8c34 |
PagedResults *prp = NULL;
|
|
|
dc8c34 |
@@ -753,7 +755,7 @@ pagedresults_is_timedout(Connection *conn)
|
|
|
dc8c34 |
|
|
|
dc8c34 |
/* reset all timeout */
|
|
|
dc8c34 |
int
|
|
|
dc8c34 |
-pagedresults_reset_timedout(Connection *conn)
|
|
|
dc8c34 |
+pagedresults_reset_timedout_nolock(Connection *conn)
|
|
|
dc8c34 |
{
|
|
|
dc8c34 |
int i;
|
|
|
dc8c34 |
PagedResults *prp = NULL;
|
|
|
dc8c34 |
@@ -773,7 +775,7 @@ pagedresults_reset_timedout(Connection *conn)
|
|
|
dc8c34 |
|
|
|
dc8c34 |
/* paged results requests are in progress. */
|
|
|
dc8c34 |
int
|
|
|
dc8c34 |
-pagedresults_in_use(Connection *conn)
|
|
|
dc8c34 |
+pagedresults_in_use_nolock(Connection *conn)
|
|
|
dc8c34 |
{
|
|
|
dc8c34 |
LDAPDebug0Args(LDAP_DEBUG_TRACE, "--> pagedresults_in_use\n");
|
|
|
dc8c34 |
if (NULL == conn) {
|
|
|
dc8c34 |
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
|
|
|
dc8c34 |
index 1b62c13..7e438b7 100644
|
|
|
dc8c34 |
--- a/ldap/servers/slapd/proto-slap.h
|
|
|
dc8c34 |
+++ b/ldap/servers/slapd/proto-slap.h
|
|
|
dc8c34 |
@@ -1421,11 +1421,11 @@ int pagedresults_set_timelimit(Connection *conn, time_t timelimit, int index);
|
|
|
dc8c34 |
int pagedresults_cleanup(Connection *conn, int needlock);
|
|
|
dc8c34 |
int pagedresults_check_or_set_processing(Connection *conn, int index);
|
|
|
dc8c34 |
int pagedresults_reset_processing(Connection *conn, int index);
|
|
|
dc8c34 |
-int pagedresults_is_timedout(Connection *conn);
|
|
|
dc8c34 |
-int pagedresults_reset_timedout(Connection *conn);
|
|
|
dc8c34 |
-int pagedresults_in_use(Connection *conn);
|
|
|
dc8c34 |
+int pagedresults_is_timedout_nolock(Connection *conn);
|
|
|
dc8c34 |
+int pagedresults_reset_timedout_nolock(Connection *conn);
|
|
|
dc8c34 |
+int pagedresults_in_use_nolock(Connection *conn);
|
|
|
dc8c34 |
int pagedresults_free_one(Connection *conn, int index);
|
|
|
dc8c34 |
-int pagedresults_free_one_msgid( Connection *conn, ber_int_t msgid );
|
|
|
dc8c34 |
+int pagedresults_free_one_msgid_nolock( Connection *conn, ber_int_t msgid );
|
|
|
dc8c34 |
int op_is_pagedresults(Operation *op);
|
|
|
dc8c34 |
int pagedresults_cleanup_all(Connection *conn, int needlock);
|
|
|
dc8c34 |
void op_set_pagedresults(Operation *op);
|
|
|
dc8c34 |
--
|
|
|
dc8c34 |
1.7.7.6
|
|
|
dc8c34 |
|