Fixed slapd-ldap/meta session reuse issue (ITS#3415)

Fixed slapd-monitor thread issue (ITS#3418)

diff --git a/CHANGES b/CHANGES
index e46bf63c17da347807d5c8946d3a2054e0a7003c..470b8230f5108d01fc6fac68051b40170d75b432 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,9 @@
 OpenLDAP 2.3 Change Log
 
+OpenLDAP 2.3.17 Engineering
+       Fixed slapd-ldap/meta session reuse issue (ITS#3415)
+       Fixed slapd-monitor thread issue (ITS#3418)
+
 OpenLDAP 2.3.16 Release
        Fixed slapd-bdb reindexing via cn=config not noticed issue (ITS#4260)
        Fixed slapd-monitor connection search crash (ITS#4300)

diff --git a/build/version.var b/build/version.var
index 76794ba403dd19c5bd0affd5a2f558e1061d6869..59554f5b92960904ac8c520627716343cf0c3f44 100644 (file)
--- a/build/version.var
+++ b/build/version.var
@@ -15,7 +15,7 @@
 ol_package=OpenLDAP
 ol_major=2
 ol_minor=3
-ol_patch=16
+ol_patch=X
 ol_api_inc=20316
 ol_api_current=2
 ol_api_revision=4

diff --git a/servers/slapd/back-ldap/back-ldap.h b/servers/slapd/back-ldap/back-ldap.h
index 5e243bc3d68620a027384bf387fcf29396bbf8b9..dd16d53d05f5b769fdbcc4788b2e5441df75c8d3 100644 (file)
--- a/servers/slapd/back-ldap/back-ldap.h
+++ b/servers/slapd/back-ldap/back-ldap.h
@@ -59,6 +59,7 @@ typedef struct ldapconn_t {
 #define        LDAP_BACK_FCONN_ISBMASK (LDAP_BACK_FCONN_ISBOUND|LDAP_BACK_FCONN_ISANON)
 #define        LDAP_BACK_FCONN_ISPRIV  (0x04)
 #define        LDAP_BACK_FCONN_ISTLS   (0x08)
+#define        LDAP_BACK_FCONN_BINDING (0x10)
 
 #define        LDAP_BACK_CONN_ISBOUND(lc)              LDAP_BACK_CONN_ISSET((lc), LDAP_BACK_FCONN_ISBOUND)
 #define        LDAP_BACK_CONN_ISBOUND_SET(lc)          LDAP_BACK_CONN_SET((lc), LDAP_BACK_FCONN_ISBOUND)
@@ -76,6 +77,9 @@ typedef struct ldapconn_t {
 #define        LDAP_BACK_CONN_ISTLS_SET(lc)            LDAP_BACK_CONN_SET((lc), LDAP_BACK_FCONN_ISTLS)
 #define        LDAP_BACK_CONN_ISTLS_CLEAR(lc)          LDAP_BACK_CONN_CLEAR((lc), LDAP_BACK_FCONN_ISTLS)
 #define        LDAP_BACK_CONN_ISTLS_CPY(lc, mlc)       LDAP_BACK_CONN_CPY((lc), LDAP_BACK_FCONN_ISTLS, (mlc))
+#define        LDAP_BACK_CONN_BINDING(lc)              LDAP_BACK_CONN_ISSET((lc), LDAP_BACK_FCONN_BINDING)
+#define        LDAP_BACK_CONN_BINDING_SET(lc)          LDAP_BACK_CONN_SET((lc), LDAP_BACK_FCONN_BINDING)
+#define        LDAP_BACK_CONN_BINDING_CLEAR(lc)        LDAP_BACK_CONN_CLEAR((lc), LDAP_BACK_FCONN_BINDING)
 
        unsigned                lc_refcnt;
        unsigned                lc_flags;
@@ -193,7 +197,12 @@ typedef enum ldap_back_send_t {
        LDAP_BACK_DONTSEND              = 0x00,
        LDAP_BACK_SENDOK                = 0x01,
        LDAP_BACK_SENDERR               = 0x02,
-       LDAP_BACK_SENDRESULT            = (LDAP_BACK_SENDOK|LDAP_BACK_SENDERR)
+       LDAP_BACK_SENDRESULT            = (LDAP_BACK_SENDOK|LDAP_BACK_SENDERR),
+       LDAP_BACK_BINDING               = 0x04,
+       LDAP_BACK_BIND_DONTSEND         = (LDAP_BACK_BINDING),
+       LDAP_BACK_BIND_SOK              = (LDAP_BACK_BINDING|LDAP_BACK_SENDOK),
+       LDAP_BACK_BIND_SERR             = (LDAP_BACK_BINDING|LDAP_BACK_SENDERR),
+       LDAP_BACK_BIND_SRES             = (LDAP_BACK_BINDING|LDAP_BACK_SENDRESULT)
 } ldap_back_send_t;
 
 /* define to use asynchronous StartTLS */

diff --git a/servers/slapd/back-ldap/bind.c b/servers/slapd/back-ldap/bind.c
index cadcaa8b05a9c7038e85dc1ad16dfbf2fa8367e9..54004ebee48c77596c57cb6246b86e887fca28ad 100644 (file)
--- a/servers/slapd/back-ldap/bind.c
+++ b/servers/slapd/back-ldap/bind.c
@@ -56,7 +56,7 @@ ldap_back_bind( Operation *op, SlapReply *rs )
        int rc = 0;
        ber_int_t msgid;
 
-       lc = ldap_back_getconn( op, rs, LDAP_BACK_SENDERR );
+       lc = ldap_back_getconn( op, rs, LDAP_BACK_BIND_SERR );
        if ( !lc ) {
                return rs->sr_err;
        }
@@ -468,7 +468,7 @@ ldapconn_t *
 ldap_back_getconn( Operation *op, SlapReply *rs, ldap_back_send_t sendok )
 {
        ldapinfo_t      *li = (ldapinfo_t *)op->o_bd->be_private;
-       ldapconn_t      *lc,
+       ldapconn_t      *lc = NULL,
                        lc_curr = { 0 };
        int             refcnt = 1;
 
@@ -489,22 +489,34 @@ ldap_back_getconn( Operation *op, SlapReply *rs, ldap_back_send_t sendok )
                }
        }
 
-       /* Searches for a ldapconn in the avl tree */
-       ldap_pvt_thread_mutex_lock( &li->li_conninfo.lai_mutex );
+       /* Explicit Bind requests always get their own conn */
+       if ( !( sendok & LDAP_BACK_BINDING ) ) {
+               /* Searches for a ldapconn in the avl tree */
+retry_lock:
+               ldap_pvt_thread_mutex_lock( &li->li_conninfo.lai_mutex );
 
-       lc = (ldapconn_t *)avl_find( li->li_conninfo.lai_tree, 
-                       (caddr_t)&lc_curr, ldap_back_conn_cmp );
-       if ( lc != NULL ) {
-               refcnt = ++lc->lc_refcnt;
+               lc = (ldapconn_t *)avl_find( li->li_conninfo.lai_tree, 
+                               (caddr_t)&lc_curr, ldap_back_conn_cmp );
+               if ( lc != NULL ) {
+                       /* Don't reuse connections while they're still binding */
+                       if ( LDAP_BACK_CONN_BINDING( lc ) ) {
+                               ldap_pvt_thread_mutex_unlock( &li->li_conninfo.lai_mutex );
+                               ldap_pvt_thread_yield();
+                               goto retry_lock;
+                       }
+                       refcnt = ++lc->lc_refcnt;
+               }
+               ldap_pvt_thread_mutex_unlock( &li->li_conninfo.lai_mutex );
        }
-       ldap_pvt_thread_mutex_unlock( &li->li_conninfo.lai_mutex );
 
        /* Looks like we didn't get a bind. Open a new session... */
        if ( lc == NULL ) {
                if ( ldap_back_prepare_conn( &lc, op, rs, sendok ) != LDAP_SUCCESS ) {
                        return NULL;
                }
-
+               if ( sendok & LDAP_BACK_BINDING ) {
+                       LDAP_BACK_CONN_BINDING_SET( lc );
+               }
                lc->lc_conn = lc_curr.lc_conn;
                ber_dupbv( &lc->lc_local_ndn, &lc_curr.lc_local_ndn );
 
@@ -618,6 +630,7 @@ ldap_back_release_conn(
        ldap_pvt_thread_mutex_lock( &li->li_conninfo.lai_mutex );
        assert( lc->lc_refcnt > 0 );
        lc->lc_refcnt--;
+       LDAP_BACK_CONN_BINDING_CLEAR( lc );
        ldap_pvt_thread_mutex_unlock( &li->li_conninfo.lai_mutex );
 }
 
@@ -650,6 +663,12 @@ ldap_back_dobind_int(
                return rc;
        }
 
+       while ( lc->lc_refcnt > 1 ) {
+               ldap_pvt_thread_yield();
+               if (( rc = LDAP_BACK_CONN_ISBOUND( lc )))
+                       return rc;
+       }
+
        /*
         * FIXME: we need to let clients use proxyAuthz
         * otherwise we cannot do symmetric pools of servers;
@@ -1386,8 +1405,9 @@ ldap_back_proxy_authz_ctrl(
                        /* just count ctrls */ ;
        }
 
-       ctrls = ch_malloc( sizeof( LDAPControl * ) * (i + 2) );
-       ctrls[ 0 ] = ch_malloc( sizeof( LDAPControl ) );
+       ctrls = op->o_tmpalloc( sizeof( LDAPControl * ) * (i + 2) + sizeof( LDAPControl ),
+                       op->o_tmpmemctx );
+       ctrls[ 0 ] = (LDAPControl *)&ctrls[ i + 2 ];
        
        ctrls[ 0 ]->ldctl_oid = LDAP_CONTROL_PROXY_AUTHZ;
        ctrls[ 0 ]->ldctl_iscritical = 1;
@@ -1396,13 +1416,14 @@ ldap_back_proxy_authz_ctrl(
        /* already in u:ID or dn:DN form */
        case LDAP_BACK_IDASSERT_OTHERID:
        case LDAP_BACK_IDASSERT_OTHERDN:
-               ber_dupbv( &ctrls[ 0 ]->ldctl_value, &assertedID );
+               ber_dupbv_x( &ctrls[ 0 ]->ldctl_value, &assertedID, op->o_tmpmemctx );
                break;
 
        /* needs the dn: prefix */
        default:
                ctrls[ 0 ]->ldctl_value.bv_len = assertedID.bv_len + STRLENOF( "dn:" );
-               ctrls[ 0 ]->ldctl_value.bv_val = ch_malloc( ctrls[ 0 ]->ldctl_value.bv_len + 1 );
+               ctrls[ 0 ]->ldctl_value.bv_val = op->o_tmpalloc( ctrls[ 0 ]->ldctl_value.bv_len + 1,
+                               op->o_tmpmemctx );
                AC_MEMCPY( ctrls[ 0 ]->ldctl_value.bv_val, "dn:", STRLENOF( "dn:" ) );
                AC_MEMCPY( &ctrls[ 0 ]->ldctl_value.bv_val[ STRLENOF( "dn:" ) ],
                                assertedID.bv_val, assertedID.bv_len + 1 );
@@ -1438,11 +1459,10 @@ ldap_back_proxy_authz_ctrl_free( Operation *op, LDAPControl ***pctrls )
                assert( ctrls[ 0 ] != NULL );
 
                if ( !BER_BVISNULL( &ctrls[ 0 ]->ldctl_value ) ) {
-                       free( ctrls[ 0 ]->ldctl_value.bv_val );
+                       op->o_tmpfree( ctrls[ 0 ]->ldctl_value.bv_val, op->o_tmpmemctx );
                }
 
-               free( ctrls[ 0 ] );
-               free( ctrls );
+               op->o_tmpfree( ctrls, op->o_tmpmemctx );
        } 
 
        *pctrls = NULL;

diff --git a/servers/slapd/back-meta/bind.c b/servers/slapd/back-meta/bind.c
index 54c669e7fcd54431f764b99757ccce30b90809b3..32389a75c15adaf5da917c4cde0fb127be27d90c 100644 (file)
--- a/servers/slapd/back-meta/bind.c
+++ b/servers/slapd/back-meta/bind.c
@@ -91,7 +91,7 @@ meta_back_bind( Operation *op, SlapReply *rs )
        /* we need meta_back_getconn() not send result even on error,
         * because we want to intercept the error and make it
         * invalidCredentials */
-       mc = meta_back_getconn( op, rs, NULL, LDAP_BACK_DONTSEND );
+       mc = meta_back_getconn( op, rs, NULL, LDAP_BACK_BIND_DONTSEND );
        if ( !mc ) {
                char    buf[ SLAP_TEXT_BUFLEN ];
 

diff --git a/servers/slapd/back-meta/conn.c b/servers/slapd/back-meta/conn.c
index b92b285f2891fae21bc78e31251204cd8b10a33d..08fcbea92c0f71447d372c62bda9d01a9079e112 100644 (file)
--- a/servers/slapd/back-meta/conn.c
+++ b/servers/slapd/back-meta/conn.c
@@ -755,21 +755,31 @@ meta_back_getconn(
                }
        }
 
-       /* Searches for a metaconn in the avl tree */
-       ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex );
-       mc = (metaconn_t *)avl_find( mi->mi_conninfo.lai_tree, 
-               (caddr_t)&mc_curr, meta_back_conn_cmp );
-       if ( mc ) {
-               if ( mc->mc_tainted ) {
-                       rs->sr_err = LDAP_UNAVAILABLE;
-                       rs->sr_text = "remote server unavailable";
-                       ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex );
-                       return NULL;
-               }
+       /* Explicit Bind requests always get their own conn */
+       if ( !( sendok & LDAP_BACK_BINDING ) ) {
+               /* Searches for a metaconn in the avl tree */
+retry_lock:
+               ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex );
+               mc = (metaconn_t *)avl_find( mi->mi_conninfo.lai_tree, 
+                       (caddr_t)&mc_curr, meta_back_conn_cmp );
+               if ( mc ) {
+                       if ( mc->mc_tainted ) {
+                               rs->sr_err = LDAP_UNAVAILABLE;
+                               rs->sr_text = "remote server unavailable";
+                               ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex );
+                               return NULL;
+                       }
                        
-               mc->mc_refcnt++;
+                       /* Don't reuse connections while they're still binding */
+                       if ( LDAP_BACK_CONN_BINDING( mc ) ) {
+                               ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex );
+                               ldap_pvt_thread_yield();
+                               goto retry_lock;
+                       }
+                       mc->mc_refcnt++;
+               }
+               ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex );
        }
-       ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex );
 
        switch ( op->o_tag ) {
        case LDAP_REQ_ADD:
@@ -822,6 +832,9 @@ meta_back_getconn(
                        mc->mc_conn = mc_curr.mc_conn;
                        ber_dupbv( &mc->mc_local_ndn, &mc_curr.mc_local_ndn );
                        new_conn = 1;
+                       if ( sendok & LDAP_BACK_BINDING ) {
+                               LDAP_BACK_CONN_BINDING_SET( mc );
+                       }
                }
 
                for ( i = 0; i < mi->mi_ntargets; i++ ) {
@@ -947,13 +960,15 @@ meta_back_getconn(
                        /* Retries searching for a metaconn in the avl tree
                         * the reason is that the connection might have been
                         * created by meta_back_get_candidate() */
-                       ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex );
-                       mc = (metaconn_t *)avl_find( mi->mi_conninfo.lai_tree, 
-                               (caddr_t)&mc_curr, meta_back_conn_cmp );
-                       if ( mc != NULL ) {
-                               mc->mc_refcnt++;
+                       if ( !( sendok & LDAP_BACK_BINDING ) ) {
+                               ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex );
+                               mc = (metaconn_t *)avl_find( mi->mi_conninfo.lai_tree, 
+                                       (caddr_t)&mc_curr, meta_back_conn_cmp );
+                               if ( mc != NULL ) {
+                                       mc->mc_refcnt++;
+                               }
+                               ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex );
                        }
-                       ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex );
 
                        /* Looks like we didn't get a bind. Open a new session... */
                        if ( mc == NULL ) {
@@ -961,6 +976,9 @@ meta_back_getconn(
                                mc->mc_conn = mc_curr.mc_conn;
                                ber_dupbv( &mc->mc_local_ndn, &mc_curr.mc_local_ndn );
                                new_conn = 1;
+                               if ( sendok & LDAP_BACK_BINDING ) {
+                                       LDAP_BACK_CONN_BINDING_SET( mc );
+                               }
                        }
                }
 
@@ -1015,6 +1033,9 @@ meta_back_getconn(
                        mc->mc_conn = mc_curr.mc_conn;
                        ber_dupbv( &mc->mc_local_ndn, &mc_curr.mc_local_ndn );
                        new_conn = 1;
+                       if ( sendok & LDAP_BACK_BINDING ) {
+                               LDAP_BACK_CONN_BINDING_SET( mc );
+                       }
                }
 
                for ( i = 0; i < mi->mi_ntargets; i++ ) {
@@ -1165,6 +1186,7 @@ meta_back_release_conn(
        ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex );
        assert( mc->mc_refcnt > 0 );
        mc->mc_refcnt--;
+       LDAP_BACK_CONN_BINDING_CLEAR( mc );
        if ( mc->mc_refcnt == 0 && mc->mc_tainted ) {
                (void)avl_delete( &mi->mi_conninfo.lai_tree, ( caddr_t )mc,
                                meta_back_conn_cmp );

diff --git a/servers/slapd/back-monitor/thread.c b/servers/slapd/back-monitor/thread.c
index 465e8645193001285ec906b38246197b20464b22..2e4ded453561b80228588b95535e49f4996a75e1 100644 (file)
--- a/servers/slapd/back-monitor/thread.c
+++ b/servers/slapd/back-monitor/thread.c
@@ -271,9 +271,9 @@ monitor_subsys_thread_update(
                break;
 
        case 2:
-               for ( i = 0; !BER_BVISNULL( a->a_vals + i ); i++) {
+               for ( i = 0; !BER_BVISNULL( &a->a_vals[ i ] ); i++ ) {
                        ch_free( a->a_vals[i].bv_val );
-                       BER_BVZERO( a->a_vals + i );
+                       BER_BVZERO( &a->a_vals[ i ] );
                }
                bv.bv_val = buf;
                ldap_pvt_thread_mutex_lock( &slapd_rq.rq_mutex );
@@ -283,6 +283,12 @@ monitor_subsys_thread_update(
                        value_add_one( &a->a_vals, &bv );
                }
                ldap_pvt_thread_mutex_unlock( &slapd_rq.rq_mutex );
+
+               /* don't leave 'round attributes with no values */
+               if ( BER_BVISNULL( &a->a_vals[ 0 ] ) ) {
+                       BER_BVSTR( &bv, "()" );
+                       value_add_one( &a->a_vals, &bv );
+               }
                break;
        }