ITS#5853 restructure wait4msg / try_read1msg again. Consolidate

the two try_read1msg cases into one, bump refcnts to prevent
lconn's from being freed prematurely.

diff --git a/libraries/libldap/os-ip.c b/libraries/libldap/os-ip.c
index bebd82382ffcf0fc5c040ef2764ca9250326e6ad..3fc414564b0f6fe108e632772c8b76d948ab5dd3 100644 (file)
--- a/libraries/libldap/os-ip.c
+++ b/libraries/libldap/os-ip.c
@@ -957,6 +957,9 @@ ldap_is_read_ready( LDAP *ld, Sockbuf *sb )
 
        sip = (struct selectinfo *)ld->ld_selectinfo;
 
+       if (ber_sockbuf_ctrl( sb, LBER_SB_OPT_DATA_READY, NULL ))
+               return 1;
+
        ber_sockbuf_ctrl( sb, LBER_SB_OPT_GET_FD, &sd );
 
 #ifdef HAVE_POLL

diff --git a/libraries/libldap/result.c b/libraries/libldap/result.c
index 10ca5b7d3c4131a90c7501827b9246fbb7d5c0f2..21f02c9831b338b862801ebcefabc736352b0ee9 100644 (file)
--- a/libraries/libldap/result.c
+++ b/libraries/libldap/result.c
@@ -71,7 +71,7 @@ static int ldap_mark_abandoned LDAP_P(( LDAP *ld, ber_int_t msgid, int idx ));
 static int wait4msg LDAP_P(( LDAP *ld, ber_int_t msgid, int all, struct timeval *timeout,
        LDAPMessage **result ));
 static ber_tag_t try_read1msg LDAP_P(( LDAP *ld, ber_int_t msgid,
-       int all, LDAPConn **lc, LDAPMessage **result ));
+       int all, LDAPConn *lc, LDAPMessage **result ));
 static ber_tag_t build_result_ber LDAP_P(( LDAP *ld, BerElement **bp, LDAPRequest *lr ));
 static void merge_error_info LDAP_P(( LDAP *ld, LDAPRequest *parentr, LDAPRequest *lr ));
 static LDAPMessage * chkResponseList LDAP_P(( LDAP *ld, int msgid, int all));
@@ -106,7 +106,7 @@ ldap_result(
        struct timeval *timeout,
        LDAPMessage **result )
 {
-       LDAPMessage     *lm = NULL;
+       LDAPMessage     *lm;
        int             rc;
 
        assert( ld != NULL );
@@ -118,19 +118,7 @@ ldap_result(
        ldap_pvt_thread_mutex_lock( &ld->ld_res_mutex );
 #endif
 
-#if 0
-       /* this is already done inside wait4msg(), right?... */
-       lm = chkResponseList( ld, msgid, all );
-#endif
-
-       if ( lm == NULL ) {
-               rc = wait4msg( ld, msgid, all, timeout, result );
-
-       } else {
-               *result = lm;
-               ld->ld_errno = LDAP_SUCCESS;
-               rc = lm->lm_msgtype;
-       }
+       rc = wait4msg( ld, msgid, all, timeout, result );
 
 #ifdef LDAP_R_COMPILE
        ldap_pvt_thread_mutex_unlock( &ld->ld_res_mutex );
@@ -335,13 +323,6 @@ wait4msg(
                                if ( ber_sockbuf_ctrl( lc->lconn_sb,
                                        LBER_SB_OPT_DATA_READY, NULL ) )
                                {
-#ifdef LDAP_R_COMPILE
-                                       ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
-#endif
-                                       rc = try_read1msg( ld, msgid, all, &lc, result );
-#ifdef LDAP_R_COMPILE
-                                       ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
-#endif
                                        lc_ready = 1;
                                        break;
                                }
@@ -375,54 +356,63 @@ wait4msg(
                                        rc = LDAP_MSG_X_KEEP_LOOKING;   /* select interrupted: loop */
 
                                } else {
-                                       rc = LDAP_MSG_X_KEEP_LOOKING;
+                                       lc_ready = 1;
+                               }
+                       }
+                       if ( lc_ready ) {
+                               LDAPConn *lnext;
+                               rc = LDAP_MSG_X_KEEP_LOOKING;
 #ifdef LDAP_R_COMPILE
-                                       ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
+                               ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
 #endif
-                                       if ( ld->ld_requests &&
-                                               ld->ld_requests->lr_status == LDAP_REQST_WRITING &&
-                                               ldap_is_write_ready( ld,
-                                                       ld->ld_requests->lr_conn->lconn_sb ) )
-                                       {
-                                               ldap_int_flush_request( ld, ld->ld_requests );
-                                       }
+                               if ( ld->ld_requests &&
+                                       ld->ld_requests->lr_status == LDAP_REQST_WRITING &&
+                                       ldap_is_write_ready( ld,
+                                               ld->ld_requests->lr_conn->lconn_sb ) )
+                               {
+                                       ldap_int_flush_request( ld, ld->ld_requests );
+                               }
 #ifdef LDAP_R_COMPILE
-                                       ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
-                                       ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
+                               ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
+                               ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
 #endif
-                                       for ( lc = ld->ld_conns;
-                                               rc == LDAP_MSG_X_KEEP_LOOKING && lc != NULL; )
+                               for ( lc = ld->ld_conns;
+                                       rc == LDAP_MSG_X_KEEP_LOOKING && lc != NULL;
+                                       lc = lnext )
+                               {
+                                       if ( lc->lconn_status == LDAP_CONNST_CONNECTED &&
+                                               ldap_is_read_ready( ld, lc->lconn_sb ) )
                                        {
-                                               if ( lc->lconn_status == LDAP_CONNST_CONNECTED &&
-                                                       ldap_is_read_ready( ld, lc->lconn_sb ) )
-                                               {
+                                               /* Don't let it get freed out from under us */
+                                               ++lc->lconn_refcnt;
 #ifdef LDAP_R_COMPILE
-                                                       ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
+                                               ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
 #endif
-                                                       rc = try_read1msg( ld, msgid, all, &lc, result );
+                                               rc = try_read1msg( ld, msgid, all, lc, result );
+                                               lnext = lc->lconn_next;
+
+                                               /* Only take locks if we're really freeing */
+                                               if ( lc->lconn_refcnt <= 1 ) {
 #ifdef LDAP_R_COMPILE
-                                                       ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
+                                                       ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
 #endif
-                                                       if ( lc == NULL ) {
-                                                               /* if lc gets free()'d,
-                                                                * there's no guarantee
-                                                                * lc->lconn_next is still
-                                                                * sane; better restart
-                                                                * (ITS#4405) */
-                                                               lc = ld->ld_conns;
-
-                                                               /* don't get to next conn! */
-                                                               break;
-                                                       }
+                                                       ldap_free_connection( ld, lc, 0, 1 );
+#ifdef LDAP_R_COMPILE
+                                                       ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
+#endif
+                                               } else {
+                                                       --lc->lconn_refcnt;
                                                }
-
-                                               /* next conn */
-                                               lc = lc->lconn_next;
-                                       }
 #ifdef LDAP_R_COMPILE
-                                       ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
+                                               ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
 #endif
+                                       } else {
+                                               lnext = lc->lconn_next;
+                                       }
                                }
+#ifdef LDAP_R_COMPILE
+                               ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
+#endif
                        }
                }
 
@@ -482,7 +472,7 @@ try_read1msg(
        LDAP *ld,
        ber_int_t msgid,
        int all,
-       LDAPConn **lcp,
+       LDAPConn *lc,
        LDAPMessage **result )
 {
        BerElement      *ber;
@@ -493,7 +483,6 @@ try_read1msg(
        ber_len_t       len;
        int             foundit = 0;
        LDAPRequest     *lr, *tmplr, dummy_lr = { 0 };
-       LDAPConn        *lc;
        BerElement      tmpber;
        int             rc, refer_cnt, hadref, simple_request, err;
        ber_int_t       lderr;
@@ -504,8 +493,7 @@ try_read1msg(
 #endif
 
        assert( ld != NULL );
-       assert( lcp != NULL );
-       assert( *lcp != NULL );
+       assert( lc != NULL );
        
 #ifdef LDAP_R_COMPILE
        LDAP_PVT_THREAD_ASSERT_MUTEX_OWNER( &ld->ld_res_mutex );
@@ -514,8 +502,6 @@ try_read1msg(
        Debug( LDAP_DEBUG_TRACE, "read1msg: ld %p msgid %d all %d\n",
                (void *)ld, msgid, all );
 
-       lc = *lcp;
-
 retry:
        if ( lc->lconn_ber == NULL ) {
                lc->lconn_ber = ldap_alloc_ber_with_options( ld );
@@ -561,14 +547,8 @@ nextresp3:
                if ( err == EAGAIN ) return LDAP_MSG_X_KEEP_LOOKING;
 #endif
                ld->ld_errno = LDAP_SERVER_DOWN;
-#ifdef LDAP_R_COMPILE
-               ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
-#endif
-               ldap_free_connection( ld, lc, 1, 0 );
-#ifdef LDAP_R_COMPILE
-               ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
-#endif
-               lc = *lcp = NULL;
+               --lc->lconn_refcnt;
+               lc->lconn_status = 0;
                return -1;
 
        default:
@@ -938,14 +918,8 @@ nextresp2:
                         * shouldn't necessarily end the connection
                         */
                        if ( lc != NULL && id != 0 ) {
-#ifdef LDAP_R_COMPILE
-                               ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
-#endif
-                               ldap_free_connection( ld, lc, 0, 1 );
-#ifdef LDAP_R_COMPILE
-                               ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
-#endif
-                               lc = *lcp = NULL;
+                               --lc->lconn_refcnt;
+                               lc = NULL;
                        }
                }
        }
@@ -1011,14 +985,7 @@ nextresp2:
 
                        /* get rid of the connection... */
                        if ( lc != NULL ) {
-#ifdef LDAP_R_COMPILE
-                               ldap_pvt_thread_mutex_lock( &ld->ld_req_mutex );
-#endif
-                               ldap_free_connection( ld, lc, 0, 1 );
-#ifdef LDAP_R_COMPILE
-                               ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
-#endif
-                               lc = *lcp = NULL;
+                               --lc->lconn_refcnt;
                        }
 
                        /* need to return -1, because otherwise