ldap/servers/slapd/back-ldbm/ldbm_search.c | 22 +++++++++------
ldap/servers/slapd/connection.c | 5 ++-
ldap/servers/slapd/daemon.c | 40 ++++++++++++++++-------------
ldap/servers/slapd/opshared.c | 23 ++++++++++++++--
ldap/servers/slapd/pagedresults.c | 6 ++--
ldap/servers/slapd/proto-slap.h | 2 -
6 files changed, 63 insertions(+), 35 deletions(-)
New commits:
commit fc5db54b96e3781e598195cb3d9e231cf8599caf
Author: Noriko Hosoi <nhosoi(a)jiji.usersys.redhat.com>
Date: Thu May 26 10:26:41 2011 -0700
Bug 668619 - slapd stops responding
https://bugzilla.redhat.com/show_bug.cgi?id=668619
Description: This patch contains 2 fixes:
1) I am moving the disconnect_server_nomutex into the section
where it adds the fd to the poll list - if we are going to
disconnect the connection due to timelimit, we should not add
the fd to the poll list - we should also remove the connection
from the active list. (by rmeggins(a)redhat.com)
2) If simple paged search result request hits timelimit,
the request is abandoned and the search result set allocated in
the backend module is released. The release is executed by the
main thread, which is protected by the c_mutex (in Connection).
The search result is sent back to the client by a worker thread.
The worker thread had accessed the search result set without any
checking. This patch adds the protection using c_mutex if the
search is simple paged.
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c
b/ldap/servers/slapd/back-ldbm/ldbm_search.c
index 2ca416f..ddebaaf 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_search.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c
@@ -140,7 +140,7 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb,
slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &sr );
if ( (NULL != sr) && (function_result != 0) ) {
/* in case paged results, clean up the conn */
- pagedresults_set_search_result(pb->pb_conn, NULL);
+ pagedresults_set_search_result(pb->pb_conn, NULL, 0);
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, NULL );
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate
);
delete_search_result_set(&sr);
@@ -1160,7 +1160,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension
)
int tlimit, llimit, slimit, isroot;
struct berval **urls = NULL;
int err;
- Slapi_DN basesdn;
+ Slapi_DN basesdn = {0};
char *target_uniqueid;
int rc = 0;
int estimate = 0; /* estimated search result count */
@@ -1177,8 +1177,12 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int
use_extension )
slapi_pblock_get( pb, SLAPI_OPINITIATED_TIME, &optime );
slapi_pblock_get( pb, SLAPI_REQUESTOR_ISROOT, &isroot );
slapi_pblock_get( pb, SLAPI_SEARCH_REFERRALS, &urls );
- slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &sr );
slapi_pblock_get( pb, SLAPI_TARGET_UNIQUEID, &target_uniqueid );
+ slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &sr );
+
+ if (NULL == sr) {
+ goto bail;
+ }
if (sr->sr_current_sizelimit >= 0) {
/*
@@ -1221,10 +1225,10 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int
use_extension )
{
/* check for abandon */
- if ( slapi_op_abandoned( pb ))
+ if ( slapi_op_abandoned( pb ) || (NULL == sr) )
{
/* in case paged results, clean up the conn */
- pagedresults_set_search_result(pb->pb_conn, NULL);
+ pagedresults_set_search_result(pb->pb_conn, NULL, 1);
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, NULL );
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate
);
if ( use_extension ) {
@@ -1241,7 +1245,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension
)
if ( tlimit != -1 && curtime > stoptime )
{
/* in case paged results, clean up the conn */
- pagedresults_set_search_result(pb->pb_conn, NULL);
+ pagedresults_set_search_result(pb->pb_conn, NULL, 1);
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, NULL );
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate
);
if ( use_extension ) {
@@ -1258,7 +1262,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension
)
if ( llimit != -1 && sr->sr_lookthroughcount >= llimit )
{
/* in case paged results, clean up the conn */
- pagedresults_set_search_result(pb->pb_conn, NULL);
+ pagedresults_set_search_result(pb->pb_conn, NULL, 1);
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, NULL );
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate
);
if ( use_extension ) {
@@ -1278,7 +1282,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension
)
/* No more entries */
/* destroy back_search_result_set */
/* in case paged results, clean up the conn */
- pagedresults_set_search_result(pb->pb_conn, NULL);
+ pagedresults_set_search_result(pb->pb_conn, NULL, 1);
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, NULL );
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate
);
if ( use_extension ) {
@@ -1421,7 +1425,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension
)
if ( --slimit < 0 ) {
CACHE_RETURN( &inst->inst_cache, &e );
/* in case paged results, clean up the conn */
- pagedresults_set_search_result(pb->pb_conn, NULL);
+ pagedresults_set_search_result(pb->pb_conn, NULL, 1);
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, NULL );
slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE,
&estimate );
delete_search_result_set( &sr );
diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
index 9be1138..d6b23f5 100644
--- a/ldap/servers/slapd/connection.c
+++ b/ldap/servers/slapd/connection.c
@@ -2727,8 +2727,6 @@ disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int
opid, PRErro
if ( ( conn->c_sd != SLAPD_INVALID_SOCKET &&
conn->c_connid == opconnid ) && !(conn->c_flags & CONN_FLAG_CLOSING) )
{
- pagedresults_cleanup(conn); /* In case the connection is on pagedresult */
-
/*
* PR_Close must be called before anything else is done because
* of NSPR problem on NT which requires that the socket on which
@@ -2769,6 +2767,9 @@ disconnect_server_nomutex( Connection *conn, PRUint64 opconnid, int
opid, PRErro
conn->c_gettingber = 0;
connection_abandon_operations( conn );
+ pagedresults_cleanup(conn); /* In case the connection is on pagedresult.
+ Better to call it after the op is abandened. */
+
if (! config_check_referral_mode()) {
/*
* If any of the outstanding operations on this
diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
index cda4a04..312db6a 100644
--- a/ldap/servers/slapd/daemon.c
+++ b/ldap/servers/slapd/daemon.c
@@ -1209,29 +1209,35 @@ setup_pr_read_pds(Connection_Table *ct, PRFileDesc **n_tcps,
PRFileDesc **s_tcps
if ((!c->c_gettingber)
&& (c->c_threadnumber < max_threads_per_conn))
{
- ct->fd[count].fd = c->c_prfd;
- ct->fd[count].in_flags = SLAPD_POLL_FLAGS;
- /* slot i of the connection table is mapped to slot
- * count of the fds array */
- c->c_fdi = count;
- count++;
+ int add_fd = 1;
+ /* check timeout for PAGED RESULTS */
+ if (c->c_current_be && (c->c_timelimit > 0))
+ {
+ time_t ctime = current_time();
+ if (ctime > c->c_timelimit)
+ {
+ /* Exceeded the timelimit; disconnect the client */
+ disconnect_server_nomutex(c, c->c_connid, -1,
+ SLAPD_DISCONNECT_IO_TIMEOUT, 0);
+ connection_table_move_connection_out_of_active_list(ct,c);
+ add_fd = 0; /* do not poll on this fd */
+ }
+ }
+ if (add_fd)
+ {
+ ct->fd[count].fd = c->c_prfd;
+ ct->fd[count].in_flags = SLAPD_POLL_FLAGS;
+ /* slot i of the connection table is mapped to slot
+ * count of the fds array */
+ c->c_fdi = count;
+ count++;
+ }
}
else
{
c->c_fdi = SLAPD_INVALID_SOCKET_INDEX;
}
}
- /* check timeout for PAGED RESULTS */
- if (c->c_current_be && (c->c_timelimit > 0))
- {
- time_t ctime = current_time();
- if (ctime > c->c_timelimit)
- {
- /* Exceeded the timelimit; disconnect the client */
- disconnect_server_nomutex(c, c->c_connid, -1,
- SLAPD_DISCONNECT_IO_TIMEOUT, 0);
- }
- }
PR_Unlock( c->c_mutex );
}
c = next;
diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c
index d398cd6..3f022aa 100644
--- a/ldap/servers/slapd/opshared.c
+++ b/ldap/servers/slapd/opshared.c
@@ -600,7 +600,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
/* search result could be reset in the backend/dse */
slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr);
- pagedresults_set_search_result(pb->pb_conn, sr);
+ pagedresults_set_search_result(pb->pb_conn, sr, 0);
if (PAGEDRESULTS_SEARCH_END == pr_stat) {
/* no more entries to send in the backend */
@@ -753,7 +753,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr);
slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE,
&estimate);
if (pagedresults_set_current_be(pb->pb_conn, be) < 0 ||
- pagedresults_set_search_result(pb->pb_conn, sr) < 0 ||
+ pagedresults_set_search_result(pb->pb_conn, sr, 0) < 0 ||
pagedresults_set_search_result_count(pb->pb_conn,
curr_search_count) < 0 ||
pagedresults_set_search_result_set_size_estimate(pb->pb_conn,
@@ -1128,6 +1128,9 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
char **attrs = NULL;
unsigned int pr_stat = 0;
+ if ( NULL == pb ) {
+ return rval;
+ }
slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &attrs);
slapi_pblock_get(pb, SLAPI_SEARCH_ATTRSONLY, &attrsonly);
@@ -1137,8 +1140,23 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
{
Slapi_Entry *gerentry = NULL;
Slapi_Operation *operation;
+ int is_paged = 0;
+ slapi_pblock_get (pb, SLAPI_OPERATION, &operation);
+ is_paged = operation->o_flags & OP_FLAG_PAGED_RESULTS;
+ /*
+ * If it is paged results, the search result set could be released when
+ * it reaches the timelimit. This lock protects the search result set
+ * not to be released while sending a next entry.
+ * (see pagedresults_cleanup called from disconnect_server_nomutex)
+ */
+ if ( is_paged && pb->pb_conn && pb->pb_conn->c_mutex )
{
+ PR_Lock( pb->pb_conn->c_mutex );
+ }
rc = be->be_next_search_entry(pb);
+ if ( is_paged && pb->pb_conn && pb->pb_conn->c_mutex )
{
+ PR_Unlock( pb->pb_conn->c_mutex );
+ }
if (rc < 0)
{
/*
@@ -1159,7 +1177,6 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_ENTRY, &e);
/* Check for possible get_effective_rights control */
- slapi_pblock_get (pb, SLAPI_OPERATION, &operation);
if ( operation->o_flags & OP_FLAG_GET_EFFECTIVE_RIGHTS )
{
char *errbuf = NULL;
diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c
index 652d4e1..0ec63a6 100644
--- a/ldap/servers/slapd/pagedresults.c
+++ b/ldap/servers/slapd/pagedresults.c
@@ -212,13 +212,13 @@ pagedresults_get_search_result(Connection *conn)
}
int
-pagedresults_set_search_result(Connection *conn, void *sr)
+pagedresults_set_search_result(Connection *conn, void *sr, int locked)
{
int rc = -1;
if (conn) {
- PR_Lock(conn->c_mutex);
+ if (!locked) PR_Lock(conn->c_mutex);
conn->c_search_result_set = sr;
- PR_Unlock(conn->c_mutex);
+ if (!locked) PR_Unlock(conn->c_mutex);
rc = 0;
}
return rc;
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index 7bd94d4..e24e1ff 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -1374,7 +1374,7 @@ void pagedresults_set_response_control(Slapi_PBlock *pb, int
iscritical, ber_int
Slapi_Backend *pagedresults_get_current_be(Connection *conn);
int pagedresults_set_current_be(Connection *conn, Slapi_Backend *be);
void *pagedresults_get_search_result(Connection *conn);
-int pagedresults_set_search_result(Connection *conn, void *sr);
+int pagedresults_set_search_result(Connection *conn, void *sr, int locked);
int pagedresults_get_search_result_count(Connection *conn);
int pagedresults_set_search_result_count(Connection *conn, int cnt);
int pagedresults_get_search_result_set_size_estimate(Connection *conn);