]> git.sur5r.net Git - openldap/commitdiff
fix concurrency issue when binding before a search; rework and cleanup data structure...
authorPierangelo Masarati <ando@openldap.org>
Sat, 28 Oct 2006 16:20:59 +0000 (16:20 +0000)
committerPierangelo Masarati <ando@openldap.org>
Sat, 28 Oct 2006 16:20:59 +0000 (16:20 +0000)
servers/slapd/back-meta/back-meta.h
servers/slapd/back-meta/bind.c
servers/slapd/back-meta/candidates.c
servers/slapd/back-meta/conn.c
servers/slapd/back-meta/init.c
servers/slapd/back-meta/search.c

index 0771431096e637238147334705bf314bfb870742..3f3c933bcab3776c752c37c9f63d222daae47ae9 100644 (file)
@@ -162,12 +162,11 @@ ldap_dnattr_result_rewrite(
 
 struct metainfo_t;
 
-typedef struct metasingleconn_t {
-       int                     msc_candidate;
-#define        META_NOT_CANDIDATE      ((ber_tag_t)0x0)
-#define        META_CANDIDATE          ((ber_tag_t)0x1)
-#define        META_BINDING            ((ber_tag_t)0x2)
+#define        META_NOT_CANDIDATE              ((ber_tag_t)0x0)
+#define        META_CANDIDATE                  ((ber_tag_t)0x1)
+#define        META_BINDING                    ((ber_tag_t)0x2)
 
+typedef struct metasingleconn_t {
 #define META_CND_ISSET(rs,f)           ( ( (rs)->sr_tag & (f) ) == (f) )
 #define META_CND_SET(rs,f)             ( (rs)->sr_tag |= (f) )
 #define META_CND_CLEAR(rs,f)           ( (rs)->sr_tag &= ~(f) )
@@ -188,8 +187,6 @@ typedef struct metasingleconn_t {
        /* NOTE: lc_lcflags is redefined to msc_mscflags to reuse the macros
         * defined for back-ldap */
 #define        lc_lcflags              msc_mscflags
-
-       struct metainfo_t       *msc_info;
 } metasingleconn_t;
 
 typedef struct metaconn_t {
@@ -212,6 +209,9 @@ typedef struct metaconn_t {
        int                     mc_authz_target;
 #define META_BOUND_NONE                (-1)
 #define META_BOUND_ALL         (-2)
+
+       struct metainfo_t       *mc_info;
+
        /* supersedes the connection stuff */
        metasingleconn_t        mc_conns[ 1 ];
        /* NOTE: mc_conns must be last, because
@@ -490,7 +490,9 @@ meta_clear_unused_candidates(
 
 extern int
 meta_clear_one_candidate(
-       metasingleconn_t        *mc );
+       Operation               *op,
+       metaconn_t              *mc,
+       int                     candidate );
 
 extern int
 meta_clear_candidates(
index d2d9cb02d6e40163752ff1eaeb46c3284f2dc41c..59c5fabd35a46fd728ee3519f9383bca86035de4 100644 (file)
@@ -33,6 +33,8 @@
 #include "slap.h"
 #include "../back-ldap/back-ldap.h"
 #include "back-meta.h"
+#undef ldap_debug      /* silence a warning in ldap-int.h */
+#include "../../../libraries/libldap/ldap-int.h"
 
 #include "lutil_ldap.h"
 
@@ -392,6 +394,13 @@ retry:;
                         * because there's a pending bind that will not
                         * be acknowledged */
                        ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex );
+                       assert( LDAP_BACK_CONN_BINDING( msc ) );
+
+#if 0
+                       Debug( LDAP_DEBUG_ANY, "### %s meta_back_bind_op_result ldap_unbind_ext[%d] ld=%p\n",
+                               op->o_log_prefix, candidate, (void *)msc->msc_ld );
+#endif
+
                        ldap_unbind_ext( msc->msc_ld, NULL, NULL );
                        msc->msc_ld = NULL;
                        LDAP_BACK_CONN_BINDING_CLEAR( msc );
index 64b56cc8ada7baad0059492bb2532136c8388d56..878e6aee587b8b7c9f3cbcc7e733dbec3d9491e3 100644 (file)
@@ -181,9 +181,19 @@ meta_clear_unused_candidates(
  */
 int
 meta_clear_one_candidate(
-       metasingleconn_t        *msc )
+       Operation       *op,
+       metaconn_t      *mc,
+       int             candidate )
 {
+       metasingleconn_t        *msc = &mc->mc_conns[ candidate ];
+
        if ( msc->msc_ld ) {
+
+#if 0
+               Debug( LDAP_DEBUG_ANY, "### %s meta_clear_one_candidate ldap_unbind_ext[%d] mc=%p\n",
+                       op ? op->o_log_prefix : "", candidate, (void *)mc );
+#endif
+
                ldap_unbind_ext( msc->msc_ld, NULL, NULL );
                msc->msc_ld = NULL;
        }
@@ -214,7 +224,7 @@ meta_clear_candidates( Operation *op, metaconn_t *mc )
        int             c;
 
        for ( c = 0; c < mi->mi_ntargets; c++ ) {
-               meta_clear_one_candidate( &mc->mc_conns[ c ] );
+               meta_clear_one_candidate( op, mc, c );
        }
 
        return 0;
index 976fc7f77d6ceccd22590581a2341d80836674ef..8d226df8ada646df085cbed6ae89dc725c4085d6 100644 (file)
@@ -195,7 +195,7 @@ metaconn_alloc(
 {
        metainfo_t      *mi = ( metainfo_t * )op->o_bd->be_private;
        metaconn_t      *mc;
-       int             i, ntargets = mi->mi_ntargets;
+       int             ntargets = mi->mi_ntargets;
 
        assert( ntargets > 0 );
 
@@ -206,9 +206,7 @@ metaconn_alloc(
                return NULL;
        }
 
-       for ( i = 0; i < ntargets; i++ ) {
-               mc->mc_conns[ i ].msc_info = mi;
-       }
+       mc->mc_info = mi;
 
        mc->mc_authz_target = META_BOUND_NONE;
        mc->mc_refcnt = 1;
@@ -425,6 +423,12 @@ retry:;
                if ( rs->sr_err == LDAP_SERVER_DOWN
                                || ( rs->sr_err != LDAP_SUCCESS && LDAP_BACK_TLS_CRITICAL( mi ) ) )
                {
+
+#if 0
+                       Debug( LDAP_DEBUG_ANY, "### %s meta_back_init_one_conn(TLS) ldap_unbind_ext[%d] ld=%p\n",
+                               op->o_log_prefix, candidate, (void *)msc->msc_ld );
+#endif
+
                        ldap_unbind_ext( msc->msc_ld, NULL, NULL );
                        msc->msc_ld = NULL;
                        goto error_return;
@@ -487,6 +491,12 @@ retry:;
                        if ( ldap_back_dn_massage( &dc, &op->o_conn->c_dn,
                                                &msc->msc_bound_ndn ) )
                        {
+
+#if 0
+                               Debug( LDAP_DEBUG_ANY, "### %s meta_back_init_one_conn(rewrite) ldap_unbind_ext[%d] ld=%p\n",
+                                       op->o_log_prefix, candidate, (void *)msc->msc_ld );
+#endif
+
                                ldap_unbind_ext( msc->msc_ld, NULL, NULL );
                                msc->msc_ld = NULL;
                                goto error_return;
@@ -563,7 +573,7 @@ meta_back_retry(
                                op->o_log_prefix, candidate, buf );
                }
 
-               meta_clear_one_candidate( msc );
+               meta_clear_one_candidate( op, mc, candidate );
                LDAP_BACK_CONN_ISBOUND_CLEAR( msc );
 
                ( void )rewrite_session_delete( mt->mt_rwmap.rwm_rw, op->o_conn );
@@ -571,6 +581,8 @@ meta_back_retry(
                /* mc here must be the regular mc, reset and ready for init */
                rc = meta_back_init_one_conn( op, rs, mc, candidate,
                        LDAP_BACK_CONN_ISPRIV( mc ), sendok );
+
+               /* restore the "binding" flag, in case */
                if ( binding ) {
                        LDAP_BACK_CONN_BINDING_SET( msc );
                }
@@ -584,11 +596,19 @@ meta_back_retry(
                                "meta_back_single_dobind=%d\n",
                                op->o_log_prefix, candidate, rc );
                        if ( rc == LDAP_SUCCESS ) {
-                               if ( be_isroot( op )  ) {
+                               if ( !BER_BVISNULL( &msc->msc_bound_ndn ) &&
+                                       !BER_BVISEMPTY( &msc->msc_bound_ndn ) )
+                               {
                                        LDAP_BACK_CONN_ISBOUND_SET( msc );
+
                                } else {
                                        LDAP_BACK_CONN_ISANON_SET( msc );
                                }
+
+                               /* when bound, dispose of the "binding" flag */
+                               if ( binding ) {
+                                       LDAP_BACK_CONN_BINDING_CLEAR( msc );
+                               }
                        }
                }
        }
@@ -1184,7 +1204,8 @@ retry_lock2:;
 
                for ( i = 0; i < mi->mi_ntargets; i++ ) {
                        metatarget_t            *mt = mi->mi_targets[ i ];
-                       metasingleconn_t        *msc = &mc->mc_conns[ i ];
+
+                       META_CANDIDATE_RESET( &candidates[ i ] );
 
                        if ( i == cached 
                                || meta_back_is_candidate( mt, &op->o_req_ndn,
@@ -1221,7 +1242,7 @@ retry_lock2:;
                                         * be tried?
                                         */
                                        if ( new_conn ) {
-                                               ( void )meta_clear_one_candidate( msc );
+                                               ( void )meta_clear_one_candidate( op, mc, i );
                                        }
                                        /* leave the target candidate, but record the error for later use */
                                        candidates[ i ].sr_err = lerr;
@@ -1258,9 +1279,8 @@ retry_lock2:;
 
                        } else {
                                if ( new_conn ) {
-                                       ( void )meta_clear_one_candidate( msc );
+                                       ( void )meta_clear_one_candidate( op, mc, i );
                                }
-                               META_CANDIDATE_RESET( &candidates[ i ] );
                        }
                }
 
index 46aee301ca401745680809bbc49f2b180138453b..8bec4e3fe26bbae5e530bdafdedb25628be9bfbf 100644 (file)
@@ -170,17 +170,18 @@ meta_back_conn_free(
 {
        metaconn_t              *mc = v_mc;
        int                     ntargets;
+       Operation               op;
 
        assert( mc != NULL );
        assert( mc->mc_refcnt == 0 );
 
        /* at least one must be present... */
        assert( mc->mc_conns != NULL );
-       ntargets = mc->mc_conns[ 0 ].msc_info->mi_ntargets;
+       ntargets = mc->mc_info->mi_ntargets;
        assert( ntargets > 0 );
 
        for ( ; ntargets--; ) {
-               (void)meta_clear_one_candidate( &mc->mc_conns[ ntargets ] );
+               (void)meta_clear_one_candidate( NULL, mc, ntargets );
        }
 
        if ( !BER_BVISNULL( &mc->mc_local_ndn ) ) {
index 919cb92760d523ca85544ed855c6c3ec52a05b6f..3c34c3aa658f36d85f084e4eeccae466af7361fe 100644 (file)
@@ -110,6 +110,7 @@ meta_search_dobind_init(
                LDAP_BACK_CONN_BINDING_SET( msc );
        }
        ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex );
+
        if ( retcode != META_SEARCH_BINDING ) {
                return retcode;
        }
@@ -186,6 +187,7 @@ meta_search_dobind_init(
                        NULL, NULL, &candidates[ candidate ].sr_msgid );
        switch ( rc ) {
        case LDAP_SUCCESS:
+               assert( candidates[ candidate ].sr_msgid >= 0 );
                META_BINDING_SET( &candidates[ candidate ] );
                return META_SEARCH_BINDING;
 
@@ -194,6 +196,7 @@ down:;
                /* This is the worst thing that could happen:
                 * the search will wait until the retry is over. */
                if ( meta_back_retry( op, rs, mcp, candidate, LDAP_BACK_DONTSEND ) ) {
+                       candidates[ candidate ].sr_msgid = META_MSGID_IGNORE;
                        return META_SEARCH_CANDIDATE;
                }
 
@@ -536,7 +539,9 @@ meta_back_search( Operation *op, SlapReply *rs )
        int             rc = 0, sres = LDAP_SUCCESS;
        char            *matched = NULL;
        int             last = 0, ncandidates = 0,
-                       initial_candidates = 0, candidate_match = 0;
+                       initial_candidates = 0, candidate_match = 0,
+                       needbind = 0;
+       ldap_back_send_t        sendok = LDAP_BACK_SENDERR;
        long            i;
        dncookie        dc;
        int             is_ok = 0;
@@ -549,7 +554,8 @@ meta_back_search( Operation *op, SlapReply *rs )
         * FIXME: in case of values return filter, we might want
         * to map attrs and maybe rewrite value
         */
-       mc = meta_back_getconn( op, rs, NULL, LDAP_BACK_SENDERR );
+getconn:;
+       mc = meta_back_getconn( op, rs, NULL, sendok );
        if ( !mc ) {
                return rs->sr_err;
        }
@@ -585,9 +591,12 @@ meta_back_search( Operation *op, SlapReply *rs )
                case META_SEARCH_NOT_CANDIDATE:
                        break;
 
+               case META_SEARCH_NEED_BIND:
+                       ++needbind;
+                       /* fallthru */
+
                case META_SEARCH_CANDIDATE:
                case META_SEARCH_BINDING:
-               case META_SEARCH_NEED_BIND:
                        candidates[ i ].sr_type = REP_INTERMEDIATE;
                        ++ncandidates;
                        break;
@@ -602,6 +611,26 @@ meta_back_search( Operation *op, SlapReply *rs )
                }
        }
 
+       if ( ncandidates > 0 && needbind == ncandidates ) {
+               assert( ( sendok & LDAP_BACK_BINDING ) == 0 );
+
+               /* FIXME: better create a separate connection? */
+               sendok |= LDAP_BACK_BINDING;
+
+#if 0
+               Debug( LDAP_DEBUG_TRACE, "*** %s drop mc=%p create new connection\n",
+                       op->o_log_prefix, mc, 0 );
+#endif
+
+               meta_back_release_conn( op, mc );
+               mc = NULL;
+
+               needbind = 0;
+               ncandidates = 0;
+
+               goto getconn;
+       }
+
        initial_candidates = ncandidates;
 
        if ( LogTest( LDAP_DEBUG_TRACE ) ) {
@@ -668,7 +697,10 @@ meta_back_search( Operation *op, SlapReply *rs )
         * among the candidates
         */
        for ( rc = 0; ncandidates > 0; ) {
-               int     gotit = 0, doabandon = 0;
+               int     gotit = 0,
+                       doabandon = 0;
+
+               needbind = ncandidates;
 
                /* check time limit */
                if ( op->ors_tlimit != SLAP_NO_LIMIT
@@ -701,17 +733,11 @@ meta_back_search( Operation *op, SlapReply *rs )
                                        op->o_log_prefix, i, retcode );
 
                                switch ( retcode ) {
-                               case META_SEARCH_BINDING:
                                case META_SEARCH_NEED_BIND:
-                                       break;
+                                       needbind--;
+                                       /* fallthru */
 
-                               case META_SEARCH_NOT_CANDIDATE:
-                                       /*
-                                        * When no candidates are left,
-                                        * the outer cycle finishes
-                                        */
-                                       candidates[ i ].sr_msgid = META_MSGID_IGNORE;
-                                       --ncandidates;
+                               case META_SEARCH_BINDING:
                                        break;
 
                                case META_SEARCH_ERR:
@@ -722,6 +748,16 @@ meta_back_search( Operation *op, SlapReply *rs )
                                                op->o_private = savepriv;
                                                goto finish;
                                        }
+                                       /* fallthru */
+
+                               case META_SEARCH_NOT_CANDIDATE:
+                                       /*
+                                        * When no candidates are left,
+                                        * the outer cycle finishes
+                                        */
+                                       candidates[ i ].sr_msgid = META_MSGID_IGNORE;
+                                       assert( ncandidates > 0 );
+                                       --ncandidates;
                                        break;
 
                                case META_SEARCH_CANDIDATE:
@@ -729,14 +765,10 @@ meta_back_search( Operation *op, SlapReply *rs )
                                        switch ( meta_back_search_start( op, rs, &dc, &mc, i, candidates ) )
                                        {
                                        case META_SEARCH_CANDIDATE:
+                                               assert( candidates[ i ].sr_msgid >= 0 );
                                                break;
 
-                                               /* means that failed but onerr == continue */
-                                       case META_SEARCH_NOT_CANDIDATE:
                                        case META_SEARCH_ERR:
-                                               candidates[ i ].sr_msgid = META_MSGID_IGNORE;
-                                               --ncandidates;
-
                                                if ( META_BACK_ONERR_STOP( mi ) ) {
                                                        savepriv = op->o_private;
                                                        op->o_private = (void *)i;
@@ -744,6 +776,14 @@ meta_back_search( Operation *op, SlapReply *rs )
                                                        op->o_private = savepriv;
                                                        goto finish;
                                                }
+                                               /* fallthru */
+
+                                       case META_SEARCH_NOT_CANDIDATE:
+                                               /* means that meta_back_search_start()
+                                                * failed but onerr == continue */
+                                               candidates[ i ].sr_msgid = META_MSGID_IGNORE;
+                                               assert( ncandidates > 0 );
+                                               --ncandidates;
                                                break;
 
                                        default:
@@ -814,6 +854,7 @@ really_bad:;
                                                        assert( 0 );
 
                                                default:
+                                                       /* unrecoverable error */
                                                        rc = rs->sr_err = LDAP_OTHER;
                                                        goto finish;
                                                }
@@ -833,6 +874,7 @@ really_bad:;
                                 * the outer cycle finishes
                                 */
                                candidates[ i ].sr_msgid = META_MSGID_IGNORE;
+                               assert( ncandidates > 0 );
                                --ncandidates;
                                rs->sr_err = candidates[ i ].sr_err;
                                continue;
@@ -919,6 +961,7 @@ really_bad:;
 #endif /* ! ENABLE_REWRITE */
 
                                        /* FIXME: merge all and return at the end */
+       
                                        for ( cnt = 0; references[ cnt ]; cnt++ )
                                                ;
        
@@ -1181,6 +1224,13 @@ really_bad:;
                                                ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex );
                                                if ( LDAP_BACK_CONN_BINDING( &mc->mc_conns[ i ] ) ) {
                                                        /* if still binding, destroy */
+
+#if 0
+                                                       Debug( LDAP_DEBUG_ANY, "### %s meta_back_search(abandon) "
+                                                               "ldap_unbind_ext[%ld] ld=%p\n",
+                                                               op->o_log_prefix, i, (void *)mc->mc_conns[i].msc_ld );
+#endif
+
                                                        ldap_unbind_ext( mc->mc_conns[ i ].msc_ld, NULL, NULL );
                                                        mc->mc_conns[ i ].msc_ld = NULL;
                                                        LDAP_BACK_CONN_BINDING_CLEAR( &mc->mc_conns[ i ] );
@@ -1195,6 +1245,8 @@ really_bad:;
                                        }
 
                                        candidates[ i ].sr_msgid = META_MSGID_IGNORE;
+                                       assert( ncandidates > 0 );
+                                       --ncandidates;
                                }
                        }
 
@@ -1219,7 +1271,40 @@ really_bad:;
                                lutil_timermul( &save_tv, 2, &save_tv );
                        }
 
-                       ldap_pvt_thread_yield();
+#if 0
+                       if ( LogTest( LDAP_DEBUG_TRACE ) ) {
+                               char    buf[ BUFSIZ ];
+
+                               snprintf( buf, sizeof( buf ), "%s %ld.%06ld %d/%d mc=%p",
+                                       op->o_log_prefix, save_tv.tv_sec, save_tv.tv_usec,
+                                       ncandidates, initial_candidates, mc );
+                               Debug( LDAP_DEBUG_TRACE, "### %s\n", buf, 0, 0 );
+                               for ( i = 0; i < mi->mi_ntargets; i++ ) {
+                                       if ( candidates[ i ].sr_msgid == META_MSGID_IGNORE ) {
+                                               continue;
+                                       }
+                       
+                                       snprintf( buf, sizeof( buf ), "[%ld] ld=%p%s%s\n",
+                                               i,
+                                               mc->mc_conns[ i ].msc_ld,
+                                               ( candidates[ i ].sr_msgid == META_MSGID_NEED_BIND ) ?  " needbind" : "",
+                                               META_IS_BINDING( &candidates[ i ] ) ? " binding" : "" );
+                                       Debug( LDAP_DEBUG_TRACE, "###    %s\n", buf, 0, 0 );
+                               }
+                       }
+#endif
+
+                       if ( needbind == 0 ) {
+#if 0
+                               Debug( LDAP_DEBUG_TRACE, "### %s select(%ld.%06ld)\n",
+                                       op->o_log_prefix, save_tv.tv_sec, save_tv.tv_usec );
+#endif
+                               tv = save_tv;
+                               (void)select( 0, NULL, NULL, NULL, &tv );
+
+                       } else {
+                               ldap_pvt_thread_yield();
+                       }
                }
        }
 
@@ -1364,6 +1449,12 @@ finish:;
                                assert( candidates[ i ].sr_msgid >= 0 );
                                assert( mc->mc_conns[ i ].msc_ld != NULL );
 
+#if 0
+                               Debug( LDAP_DEBUG_ANY, "### %s meta_back_search(cleanup) "
+                                       "ldap_unbind_ext[%ld] ld=%p\n",
+                                       op->o_log_prefix, i, (void *)mc->mc_conns[i].msc_ld );
+#endif
+
                                /* if still binding, destroy */
                                ldap_unbind_ext( mc->mc_conns[ i ].msc_ld, NULL, NULL );
                                mc->mc_conns[ i ].msc_ld = NULL;