From ea6f5bad6548ecc0775574b36e906518a7aa8bc9 Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Wed, 25 Aug 2004 11:52:55 +0000 Subject: [PATCH] This patch fixes some subtle interactions between SLAPI and syncrepl. Due to SLAPI always assigning connection and operation IDs of zero for internal operations, such operations would cause a stale contextCSN to be returned from slap_get_commit_csn(). As a result, SLAPI internal updates would be invisible to replicas until an external update was made. Also, SLAPI internal operations never called slap_graduate_commit_csn() which leaked pending CSNs. Also included in this patch is a general cleanup of some of the SLAPI code. Note that we need to use a separate mutex on conn_nextid to avoid a deadlock where a post-operation plugin tries to acquire connections_mutex, having locked the per-connection mutex, while the listener thread tries to acquire the per-connection mutex (having locked connections_mutex). connection.c needs to be fixed to acquire mutexes in the same order. --- servers/slapd/connection.c | 17 +++ servers/slapd/proto-slap.h | 1 + servers/slapd/slapi/slapi_ops.c | 213 +++++++++++++++----------------- 3 files changed, 118 insertions(+), 113 deletions(-) diff --git a/servers/slapd/connection.c b/servers/slapd/connection.c index b4b7c4e264..651fc7fae7 100644 --- a/servers/slapd/connection.c +++ b/servers/slapd/connection.c @@ -45,6 +45,8 @@ /* protected by connections_mutex */ static ldap_pvt_thread_mutex_t connections_mutex; static Connection *connections = NULL; + +static ldap_pvt_thread_mutex_t conn_nextid_mutex; static unsigned long conn_nextid = 0; /* structure state (protected by connections_mutex) */ @@ -109,6 +111,7 @@ int connections_init(void) /* should check return of every call */ ldap_pvt_thread_mutex_init( &connections_mutex ); + ldap_pvt_thread_mutex_init( &conn_nextid_mutex ); connections = (Connection *) ch_calloc( dtblsize, sizeof(Connection) ); @@ -176,6 +179,7 @@ int connections_destroy(void) connections = NULL; ldap_pvt_thread_mutex_destroy( &connections_mutex ); + ldap_pvt_thread_mutex_destroy( &conn_nextid_mutex ); return 0; } @@ -601,7 +605,9 @@ long connection_init( #endif } + ldap_pvt_thread_mutex_lock( &conn_nextid_mutex ); id = c->c_connid = conn_nextid++; + ldap_pvt_thread_mutex_unlock( &conn_nextid_mutex ); c->c_conn_state = SLAP_C_INACTIVE; c->c_struct_state = SLAP_C_USED; @@ -1893,3 +1899,14 @@ connection_fake_init( op->o_time = slap_get_time(); } + +void +connection_assign_nextid( Connection *conn ) +{ + int rc; + + ldap_pvt_thread_mutex_lock( &conn_nextid_mutex ); + conn->c_connid = conn_nextid++; + ldap_pvt_thread_mutex_unlock( &conn_nextid_mutex ); +} + diff --git a/servers/slapd/proto-slap.h b/servers/slapd/proto-slap.h index 87a62dfba8..7e1c79d99c 100644 --- a/servers/slapd/proto-slap.h +++ b/servers/slapd/proto-slap.h @@ -388,6 +388,7 @@ LDAP_SLAPD_F (void) connection_fake_init LDAP_P(( Connection *conn, Operation *op, void *threadctx )); +LDAP_SLAPD_F (void) connection_assign_nextid LDAP_P((Connection *)); /* * cr.c diff --git a/servers/slapd/slapi/slapi_ops.c b/servers/slapd/slapi/slapi_ops.c index 05764d339f..1a3aa22053 100644 --- a/servers/slapd/slapi/slapi_ops.c +++ b/servers/slapd/slapi/slapi_ops.c @@ -124,7 +124,7 @@ slapi_int_init_connection( char *DN, int OpType ) { - Connection *pConn, *c; + Connection *pConn; ber_len_t max = sockbuf_max_incoming; pConn = (Connection *) slapi_ch_calloc(1, sizeof(Connection)); @@ -149,79 +149,88 @@ slapi_int_init_connection( return (Connection *)NULL; } - c = pConn; - - /* operation object */ - c->c_pending_ops.stqh_first->o_tag = OpType; - c->c_pending_ops.stqh_first->o_protocol = LDAP_VERSION3; - c->c_pending_ops.stqh_first->o_authmech.bv_val = NULL; - c->c_pending_ops.stqh_first->o_authmech.bv_len = 0; - c->c_pending_ops.stqh_first->o_time = slap_get_time(); - c->c_pending_ops.stqh_first->o_do_not_cache = 1; - c->c_pending_ops.stqh_first->o_threadctx = ldap_pvt_thread_pool_context(); - c->c_pending_ops.stqh_first->o_tmpmemctx = NULL; - c->c_pending_ops.stqh_first->o_tmpmfuncs = &ch_mfuncs; - c->c_pending_ops.stqh_first->o_conn = c; - /* connection object */ - c->c_authmech.bv_val = NULL; - c->c_authmech.bv_len = 0; - c->c_dn.bv_val = NULL; - c->c_dn.bv_len = 0; - c->c_ndn.bv_val = NULL; - c->c_ndn.bv_len = 0; + pConn->c_authmech.bv_val = NULL; + pConn->c_authmech.bv_len = 0; + pConn->c_dn.bv_val = NULL; + pConn->c_dn.bv_len = 0; + pConn->c_ndn.bv_val = NULL; + pConn->c_ndn.bv_len = 0; - c->c_listener = &slap_unknown_listener; - ber_dupbv( &c->c_peer_domain, (struct berval *)&slap_unknown_bv ); - ber_dupbv( &c->c_peer_name, (struct berval *)&slap_unknown_bv ); + pConn->c_listener = &slap_unknown_listener; + ber_dupbv( &pConn->c_peer_domain, (struct berval *)&slap_unknown_bv ); + ber_dupbv( &pConn->c_peer_name, (struct berval *)&slap_unknown_bv ); - LDAP_STAILQ_INIT( &c->c_ops ); + LDAP_STAILQ_INIT( &pConn->c_ops ); - c->c_sasl_bind_mech.bv_val = NULL; - c->c_sasl_bind_mech.bv_len = 0; - c->c_sasl_authctx = NULL; - c->c_sasl_sockctx = NULL; - c->c_sasl_extra = NULL; + pConn->c_sasl_bind_mech.bv_val = NULL; + pConn->c_sasl_bind_mech.bv_len = 0; + pConn->c_sasl_authctx = NULL; + pConn->c_sasl_sockctx = NULL; + pConn->c_sasl_extra = NULL; - c->c_sb = ber_sockbuf_alloc( ); + pConn->c_sb = ber_sockbuf_alloc( ); - ber_sockbuf_ctrl( c->c_sb, LBER_SB_OPT_SET_MAX_INCOMING, &max ); + ber_sockbuf_ctrl( pConn->c_sb, LBER_SB_OPT_SET_MAX_INCOMING, &max ); - c->c_currentber = NULL; + pConn->c_currentber = NULL; /* should check status of thread calls */ - ldap_pvt_thread_mutex_init( &c->c_mutex ); - ldap_pvt_thread_mutex_init( &c->c_write_mutex ); - ldap_pvt_thread_cond_init( &c->c_write_cv ); + ldap_pvt_thread_mutex_init( &pConn->c_mutex ); + ldap_pvt_thread_mutex_init( &pConn->c_write_mutex ); + ldap_pvt_thread_cond_init( &pConn->c_write_cv ); - c->c_n_ops_received = 0; - c->c_n_ops_executing = 0; - c->c_n_ops_pending = 0; - c->c_n_ops_completed = 0; + ldap_pvt_thread_mutex_lock( &pConn->c_mutex ); - c->c_n_get = 0; - c->c_n_read = 0; - c->c_n_write = 0; + pConn->c_n_ops_received = 0; + pConn->c_n_ops_executing = 0; + pConn->c_n_ops_pending = 0; + pConn->c_n_ops_completed = 0; - c->c_protocol = LDAP_VERSION3; + pConn->c_n_get = 0; + pConn->c_n_read = 0; + pConn->c_n_write = 0; - c->c_activitytime = c->c_starttime = slap_get_time(); + pConn->c_protocol = LDAP_VERSION3; - c->c_connid = 0; + pConn->c_activitytime = pConn->c_starttime = slap_get_time(); - c->c_conn_state = 0x01; /* SLAP_C_ACTIVE */ - c->c_struct_state = 0x02; /* SLAP_C_USED */ + /* + * A real connection ID is required, because syncrepl associates + * pending CSNs with unique ( connection, operation ) tuples. + * Setting a fake connection ID will cause slap_get_commit_csn() + * to return a stale value. + */ + connection_assign_nextid( pConn ); + + pConn->c_conn_state = 0x01; /* SLAP_C_ACTIVE */ + pConn->c_struct_state = 0x02; /* SLAP_C_USED */ - c->c_ssf = c->c_transport_ssf = 0; - c->c_tls_ssf = 0; + pConn->c_ssf = pConn->c_transport_ssf = 0; + pConn->c_tls_ssf = 0; - backend_connection_init( c ); + backend_connection_init( pConn ); pConn->c_send_ldap_result = internal_result_v3; pConn->c_send_search_entry = internal_search_entry; pConn->c_send_ldap_extended = internal_result_ext; pConn->c_send_search_reference = internal_search_reference; + /* operation object */ + pConn->c_pending_ops.stqh_first->o_tag = OpType; + pConn->c_pending_ops.stqh_first->o_protocol = LDAP_VERSION3; + pConn->c_pending_ops.stqh_first->o_authmech.bv_val = NULL; + pConn->c_pending_ops.stqh_first->o_authmech.bv_len = 0; + pConn->c_pending_ops.stqh_first->o_time = slap_get_time(); + pConn->c_pending_ops.stqh_first->o_do_not_cache = 1; + pConn->c_pending_ops.stqh_first->o_threadctx = ldap_pvt_thread_pool_context(); + pConn->c_pending_ops.stqh_first->o_tmpmemctx = NULL; + pConn->c_pending_ops.stqh_first->o_tmpmfuncs = &ch_mfuncs; + pConn->c_pending_ops.stqh_first->o_conn = pConn; + pConn->c_pending_ops.stqh_first->o_connid = pConn->c_connid; + + ldap_pvt_thread_mutex_unlock( &pConn->c_mutex ); + return pConn; } @@ -236,6 +245,8 @@ void slapi_int_connection_destroy( Connection **pConn ) op = (Operation *)conn->c_pending_ops.stqh_first; + slap_graduate_commit_csn( op ); + if ( op->o_req_dn.bv_val != NULL ) { slapi_ch_free( (void **)&op->o_req_dn.bv_val ); } @@ -337,6 +348,7 @@ bvptr2obj_copy( */ static Entry * slapi_int_ldapmod_to_entry( + Connection *pConn, char *ldn, LDAPMod **mods ) { @@ -355,13 +367,7 @@ slapi_int_ldapmod_to_entry( const char *text = NULL; - - op = (Operation *) slapi_ch_calloc(1, sizeof(Operation)); - if ( op == NULL) { - rc = LDAP_NO_MEMORY; - goto cleanup; - } - op->o_tag = LDAP_REQ_ADD; + op = (Operation *)pConn->c_pending_ops.stqh_first; pEntry = (Entry *) ch_calloc( 1, sizeof(Entry) ); if ( pEntry == NULL) { @@ -478,8 +484,6 @@ cleanup: if ( dn.bv_val ) slapi_ch_free( (void **)&dn.bv_val ); - if ( op ) - slapi_ch_free( (void **)&op ); if ( modlist != NULL ) slap_mods_free( modlist ); if ( rc != LDAP_SUCCESS ) { @@ -512,7 +516,6 @@ slapi_delete_internal( Connection *pConn = NULL; Operation *op = NULL; Slapi_PBlock *pPB = NULL; - Slapi_PBlock *pSavePB = NULL; SlapReply rs = { REP_RESULT }; struct berval dn = BER_BVNULL; @@ -576,13 +579,10 @@ cleanup: if ( dn.bv_val ) { slapi_ch_free( (void **)&dn.bv_val ); } - if ( pConn != NULL ) { - pSavePB = pPB; - } slapi_int_connection_destroy( &pConn ); - return (pSavePB); + return pPB; #else return NULL; #endif /* LDAP_SLAPI */ @@ -591,13 +591,13 @@ cleanup: #ifdef LDAP_SLAPI static Slapi_PBlock * slapi_int_add_entry_locked( + Connection *pConn, Slapi_Entry **e, LDAPControl **controls, int log_changes ) { - Connection *pConn = NULL; Operation *op = NULL; - Slapi_PBlock *pPB = NULL, *pSavePB = NULL; + Slapi_PBlock *pPB = NULL; int manageDsaIt = 0; int isCritical; @@ -607,12 +607,6 @@ slapi_int_add_entry_locked( rs.sr_err = LDAP_PARAM_ERROR; goto cleanup; } - - pConn = slapi_int_init_connection( NULL, LDAP_REQ_ADD ); - if ( pConn == NULL ) { - rs.sr_err = LDAP_NO_MEMORY; - goto cleanup; - } if ( slapi_control_present( controls, LDAP_CONTROL_MANAGEDSAIT, NULL, &isCritical ) ) { @@ -650,18 +644,11 @@ slapi_int_add_entry_locked( } cleanup: - if ( pPB != NULL ) { slapi_pblock_set( pPB, SLAPI_PLUGIN_INTOP_RESULT, (void *)rs.sr_err ); } - if ( pConn != NULL ) { - pSavePB = pPB; - } - - slapi_int_connection_destroy( &pConn ); - - return( pSavePB ); + return( pPB ); } #endif /* LDAP_SLAPI */ @@ -672,18 +659,27 @@ slapi_add_entry_internal( int log_changes ) { #ifdef LDAP_SLAPI - Slapi_PBlock *pb; - Slapi_Entry *entry; + Slapi_PBlock *pb = NULL; + Slapi_Entry *entry = NULL; + Connection *pConn = NULL; + + pConn = slapi_int_init_connection( NULL, LDAP_REQ_ADD ); + if ( pConn == NULL ) { + return NULL; + } /* * We make a copy to avoid an entry that may be freed later * by the caller being placed in the cache. */ entry = slapi_entry_dup( e ); - pb = slapi_int_add_entry_locked( &entry, controls, log_changes ); + pb = slapi_int_add_entry_locked( pConn, &entry, controls, log_changes ); if ( entry != NULL ) { slapi_entry_free( entry ); } + + slapi_int_connection_destroy( &pConn ); + return pb; #else return NULL; @@ -699,6 +695,7 @@ slapi_add_internal( { #ifdef LDAP_SLAPI LDAPMod *pMod = NULL; + Connection *pConn = NULL; Slapi_PBlock *pb = NULL; Entry *pEntry = NULL; int i, rc = LDAP_SUCCESS; @@ -717,9 +714,12 @@ slapi_add_internal( } if ( rc == LDAP_SUCCESS ) { - pEntry = slapi_int_ldapmod_to_entry( dn, mods ); - if ( pEntry == NULL ) { - rc = LDAP_OTHER; + pConn = slapi_int_init_connection( NULL, LDAP_REQ_ADD ); + if ( pConn != NULL ) { + pEntry = slapi_int_ldapmod_to_entry( pConn, dn, mods ); + if ( pEntry == NULL ) { + rc = LDAP_OTHER; + } } } @@ -727,14 +727,16 @@ slapi_add_internal( pb = slapi_pblock_new(); slapi_pblock_set( pb, SLAPI_PLUGIN_INTOP_RESULT, (void *)rc ); } else { - pb = slapi_int_add_entry_locked( &pEntry, controls, log_changes ); + pb = slapi_int_add_entry_locked( pConn, &pEntry, controls, log_changes ); } if ( pEntry != NULL ) { - slapi_entry_free(pEntry); + slapi_entry_free( pEntry ); } - return(pb); + slapi_int_connection_destroy( &pConn ); + + return pb; #else return NULL; #endif /* LDAP_SLAPI */ @@ -766,12 +768,11 @@ slapi_modrdn_internal( Connection *pConn = NULL; Operation *op = NULL; Slapi_PBlock *pPB = NULL; - Slapi_PBlock *pSavePB = NULL; int manageDsaIt = 0; int isCritical; SlapReply rs = { REP_RESULT }; - pConn = slapi_int_init_connection( NULL, LDAP_REQ_MODRDN); + pConn = slapi_int_init_connection( NULL, LDAP_REQ_MODRDN ); if ( pConn == NULL) { rs.sr_err = LDAP_NO_MEMORY; goto cleanup; @@ -855,13 +856,9 @@ cleanup: if ( op->oq_modrdn.rs_nnewrdn.bv_val ) slapi_ch_free( (void **)&op->oq_modrdn.rs_nnewrdn.bv_val ); - if ( pConn != NULL ) { - pSavePB = pPB; - } - slapi_int_connection_destroy( &pConn ); - return( pSavePB ); + return pPB; #else return NULL; #endif /* LDAP_SLAPI */ @@ -889,7 +886,6 @@ slapi_modify_internal( Connection *pConn = NULL; Operation *op = NULL; Slapi_PBlock *pPB = NULL; - Slapi_PBlock *pSavePB = NULL; struct berval dn = BER_BVNULL; @@ -1060,13 +1056,9 @@ cleanup: if ( modlist != NULL ) slap_mods_free( modlist ); - if ( pConn != NULL ) { - pSavePB = pPB; - } - slapi_int_connection_destroy( &pConn ); - return ( pSavePB ); + return pPB; #else return NULL; #endif /* LDAP_SLAPI */ @@ -1084,8 +1076,7 @@ slapi_search_internal( #ifdef LDAP_SLAPI Connection *c; Operation *op = NULL; - Slapi_PBlock *ptr = NULL; - Slapi_PBlock *pSavePB = NULL; + Slapi_PBlock *pPB = NULL; struct berval dn = BER_BVNULL; Filter *filter=NULL; struct berval fstr = BER_BVNULL; @@ -1105,7 +1096,7 @@ slapi_search_internal( } op = (Operation *)c->c_pending_ops.stqh_first; - ptr = (Slapi_PBlock *)op->o_pb; + pPB = (Slapi_PBlock *)op->o_pb; op->o_ctrls = controls; if ( ldn != NULL ) { @@ -1234,8 +1225,8 @@ slapi_search_internal( cleanup: - if ( ptr != NULL ) - slapi_pblock_set( ptr, SLAPI_PLUGIN_INTOP_RESULT, (void *)rs.sr_err ); + if ( pPB != NULL ) + slapi_pblock_set( pPB, SLAPI_PLUGIN_INTOP_RESULT, (void *)rs.sr_err ); if ( dn.bv_val ) slapi_ch_free( (void **)&dn.bv_val ); @@ -1246,13 +1237,9 @@ cleanup: if ( an != NULL ) slapi_ch_free( (void **)&an ); - if ( c != NULL ) { - pSavePB = ptr; - } - slapi_int_connection_destroy( &c ); - return( pSavePB ); + return pPB; #else return NULL; #endif /* LDAP_SLAPI */ -- 2.39.5