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 [email protected])

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.

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 );

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

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;

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;

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;

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);