diff --git a/servers/lloadd/bind.c b/servers/lloadd/bind.c index 53af3895d8..54d62bde24 100644 --- a/servers/lloadd/bind.c +++ b/servers/lloadd/bind.c @@ -254,6 +254,20 @@ client_reset( Connection *c ) root = c->c_ops; c->c_ops = NULL; + + /* unless op->o_client_refcnt > op->o_client_live, there is noone using the + * operation from the client side and noone new will now that we've removed + * it from client's c_ops */ + if ( root ) { + TAvlnode *node = tavl_end( root, TAVL_DIR_LEFT ); + do { + Operation *op = node->avl_data; + + /* make sure it's useable after we've unlocked the connection */ + op->o_client_refcnt++; + } while ( (node = tavl_next( node, TAVL_DIR_RIGHT )) ); + } + if ( !BER_BVISNULL( &c->c_auth ) ) { ch_free( c->c_auth.bv_val ); BER_BVZERO( &c->c_auth ); @@ -264,11 +278,24 @@ client_reset( Connection *c ) } CONNECTION_UNLOCK_INCREF(c); - freed = tavl_free( root, (AVL_FREE)operation_abandon ); + if ( root ) { + TAvlnode *node = tavl_end( root, TAVL_DIR_LEFT ); + do { + Operation *op = node->avl_data; - Debug( LDAP_DEBUG_TRACE, "client_reset: " - "dropped %d operations\n", - freed ); + operation_abandon( op ); + + CONNECTION_LOCK(c); + op->o_client_refcnt--; + operation_destroy_from_client( op ); + CONNECTION_UNLOCK(c); + } while ( (node = tavl_next( node, TAVL_DIR_RIGHT )) ); + + freed = tavl_free( root, NULL ); + Debug( LDAP_DEBUG_TRACE, "client_reset: " + "dropped %d operations\n", + freed ); + } CONNECTION_LOCK_DECREF(c); } @@ -280,6 +307,7 @@ client_bind( Connection *client, Operation *op ) int rc = LDAP_SUCCESS; /* protect the Bind operation */ + op->o_client_refcnt++; tavl_delete( &client->c_ops, op, operation_client_cmp ); client->c_state = SLAP_C_BINDING; @@ -293,6 +321,8 @@ client_bind( Connection *client, Operation *op ) operation_send_reject( op, LDAP_UNAVAILABLE, "no connections available", 1 ); CONNECTION_LOCK_DECREF(client); + op->o_client_refcnt--; + operation_destroy_from_client( op ); return rc; } @@ -309,13 +339,19 @@ client_bind( Connection *client, Operation *op ) CONNECTION_LOCK_DECREF(client); if ( rc ) { + op->o_client_refcnt--; + operation_destroy_from_client( op ); CLIENT_DESTROY(client); return -1; } - rc = tavl_insert( &client->c_ops, op, operation_client_cmp, avl_dup_error ); - assert( rc == LDAP_SUCCESS ); - CLIENT_UNLOCK_OR_DESTROY(client); + if ( !--op->o_client_refcnt ) { + operation_destroy_from_client( op ); + } else { + rc = tavl_insert( + &client->c_ops, op, operation_client_cmp, avl_dup_error ); + assert( rc == LDAP_SUCCESS ); + } return rc; } diff --git a/servers/lloadd/client.c b/servers/lloadd/client.c index 6748689cc8..fc7edc59e6 100644 --- a/servers/lloadd/client.c +++ b/servers/lloadd/client.c @@ -177,10 +177,13 @@ handle_one_request( Connection *c ) break; default: if ( c->c_state == SLAP_C_BINDING ) { + op->o_client_refcnt++; CONNECTION_UNLOCK_INCREF(c); operation_send_reject( op, LDAP_PROTOCOL_ERROR, "bind in progress", 0 ); CONNECTION_LOCK_DECREF(c); + op->o_client_refcnt--; + operation_destroy_from_client( op ); return LDAP_SUCCESS; } handler = request_process; @@ -278,39 +281,40 @@ fail: void client_destroy( Connection *c ) { - TAvlnode *root, *node; - Debug( LDAP_DEBUG_CONNS, "client_destroy: " "destroying client %lu\n", c->c_connid ); - assert( c->c_read_event != NULL ); - event_del( c->c_read_event ); - event_free( c->c_read_event ); + if ( c->c_read_event ) { + event_del( c->c_read_event ); + event_free( c->c_read_event ); + } - assert( c->c_write_event != NULL ); - event_del( c->c_write_event ); - event_free( c->c_write_event ); - - root = c->c_ops; - c->c_ops = NULL; - - if ( !BER_BVISNULL( &c->c_auth ) ) { - ch_free( c->c_auth.bv_val ); + if ( c->c_write_event ) { + event_del( c->c_write_event ); + event_free( c->c_write_event ); } c->c_state = SLAP_C_INVALID; + /* FIXME: we drop c_mutex in client_reset, operation_destroy_from_upstream + * might copy op->o_client and bump c_refcnt, it is then responsible to + * call destroy_client again, does that mean that we can be triggered for + * recursion over all connections? */ + client_reset( c ); + + /* + * If we attempted to destroy any operations, we might have lent a new + * refcnt token for a thread that raced us to that, let them call us again + * later + */ + assert( c->c_refcnt >= 0 ); + if ( c->c_refcnt ) { + c->c_state = SLAP_C_CLOSING; + Debug( LDAP_DEBUG_CONNS, "client_destroy: " + "connid=%lu aborting with refcnt=%d\n", + c->c_connid, c->c_refcnt ); + CONNECTION_UNLOCK(c); + return; + } connection_destroy( c ); - - if ( !root ) return; - - /* We don't hold c_mutex anymore */ - node = tavl_end( root, TAVL_DIR_LEFT ); - do { - Operation *op = node->avl_data; - - op->o_client = NULL; - operation_abandon( op ); - } while ( (node = tavl_next( node, TAVL_DIR_RIGHT )) ); - tavl_free( root, NULL ); } diff --git a/servers/lloadd/daemon.c b/servers/lloadd/daemon.c index 94b16d83ff..19a7f17c08 100644 --- a/servers/lloadd/daemon.c +++ b/servers/lloadd/daemon.c @@ -92,6 +92,8 @@ struct evdns_base *dnsbase; static int emfile; +ldap_pvt_thread_mutex_t operation_mutex; + static volatile int waking; #define WAKE_DAEMON( l, w ) \ do { \ @@ -722,6 +724,8 @@ slapd_daemon_init( const char *urls ) Debug( LDAP_DEBUG_ARGS, "slapd_daemon_init: %s\n", urls ? urls : "" ); + ldap_pvt_thread_mutex_init( &operation_mutex ); + #ifdef HAVE_TCPD ldap_pvt_thread_mutex_init( &sd_tcpd_mutex ); #endif /* TCP Wrappers */ diff --git a/servers/lloadd/operation.c b/servers/lloadd/operation.c index b0996b2ff5..cf56c651aa 100644 --- a/servers/lloadd/operation.c +++ b/servers/lloadd/operation.c @@ -103,45 +103,256 @@ operation_upstream_cmp( const void *left, const void *right ) ( l->o_upstream_msgid > r->o_upstream_msgid ); } +/* + * Free the operation, subject to there being noone else holding a reference + * to it. + * + * Both operation_destroy_from_* functions are the same, two implementations + * exist to cater for the fact that either side (client or upstream) might + * decide to destroy it and each holds a different mutex. + * + * Due to the fact that we rely on mutexes on both connections which have a + * different timespan from the operation, we have to take the following race + * into account: + * + * Trigger + * - both operation_destroy_from_client and operation_destroy_from_upstream + * are called at the same time (each holding its mutex), several times + * before one of them finishes + * - either or both connections might have started the process of being + * destroyed + * + * We need to detect that the race has happened and only allow one of them to + * free the operation (we use o_freeing != 0 to announce+detect that). + * + * In case the caller was in the process of destroying the connection and the + * race had been won by the mirror caller, it will increment c_refcnt on its + * connection and make sure to postpone the final step in + * client/upstream_destroy(). Testing o_freeing for the mirror side's token + * allows the winner to detect that it has been a party to the race and a token + * in c_refcnt has been deposited on its behalf. + */ void -operation_destroy( Operation *op ) +operation_destroy_from_client( Operation *op ) { - Connection *c; + Connection *client = op->o_client, *upstream = op->o_upstream; + Backend *b = NULL; + int race_state; + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_client: " + "op=%p attempting to release operation\n", + op ); + + /* 1. liveness/refcnt adjustment and test */ + op->o_client_refcnt -= op->o_client_live; + op->o_client_live = 0; + + if ( op->o_client_refcnt ) { + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_client: " + "op=%p not dead yet\n", + op ); + return; + } + + /* 2. Remove from the operation map and TODO adjust the pending op count */ + tavl_delete( &client->c_ops, op, operation_client_cmp ); + + /* We reset the pointer after we have cleaned up both sides, upstream would + * not have been set at all */ + if ( !op->o_upstream ) { + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_client: " + "op=%p no upstream, killing now\n", + op ); + goto doit; + } + + /* 3. Detect whether we entered a race to free op and indicate that to any + * others */ + ldap_pvt_thread_mutex_lock( &operation_mutex ); + race_state = op->o_freeing; + op->o_freeing |= SLAP_OP_FREEING_CLIENT; + ldap_pvt_thread_mutex_unlock( &operation_mutex ); + + /* 4. If we lost the race, deal with it */ + if ( race_state ) { + /* + * We have raced to destroy op and the first one to lose on this side, + * leave a refcnt token on client so we don't destroy it before the + * other side has finished (it knows we did that when it examines + * o_freeing again). + */ + if ( race_state == SLAP_OP_FREEING_UPSTREAM ) { + client->c_refcnt++; + } + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_client: " + "op=%p lost race, increasing client refcnt\n", + op ); + return; + } + CONNECTION_UNLOCK_INCREF(client); + + CONNECTION_LOCK(upstream); + /* 5. If we raced the upstream side and won, reclaim the token */ + ldap_pvt_thread_mutex_lock( &operation_mutex ); + if ( op->o_freeing & SLAP_OP_FREEING_UPSTREAM ) { + /* + * We have raced to destroy op and won. To avoid freeing the connection + * under us, a refcnt token has been left over for us on the upstream, + * decref and see whether we are in charge of freeing it + */ + upstream->c_refcnt--; + } + ldap_pvt_thread_mutex_unlock( &operation_mutex ); + + /* 6. liveness/refcnt adjustment and test */ + op->o_upstream_refcnt -= op->o_upstream_live; + op->o_upstream_live = 0; + if ( op->o_upstream_refcnt ) { + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_client: " + "op=%p other side still alive, refcnt=%d\n", + op, op->o_upstream_refcnt ); + /* There must have been no race if op is still alive */ + ldap_pvt_thread_mutex_lock( &operation_mutex ); + op->o_freeing &= ~SLAP_OP_FREEING_CLIENT; + assert( op->o_freeing == 0 ); + ldap_pvt_thread_mutex_unlock( &operation_mutex ); + CONNECTION_UNLOCK(upstream); + CONNECTION_LOCK_DECREF(client); + return; + } + + /* 7. Remove from the operation map and adjust the pending op count */ + if ( tavl_delete( &upstream->c_ops, op, operation_upstream_cmp ) ) { + upstream->c_n_ops_executing--; + b = (Backend *)upstream->c_private; + } + UPSTREAM_UNLOCK_OR_DESTROY(upstream); + + if ( b ) { + ldap_pvt_thread_mutex_lock( &b->b_mutex ); + b->b_n_ops_executing--; + ldap_pvt_thread_mutex_unlock( &b->b_mutex ); + } + + CONNECTION_LOCK_DECREF(client); + +doit: + /* 8. Release the operation */ + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_client: " + "op=%p destroyed operation from client connid=%lu, " + "client msgid=%d\n", + op, op->o_client_connid, op->o_client_msgid ); ber_free( op->o_ber, 1 ); - - /* TODO: this is a stopgap and there are many races here, just get - * something in to test with until we implement the freelist */ - if ( op->o_client ) { - c = op->o_client; - CONNECTION_LOCK(c); - if ( tavl_delete( &c->c_ops, op, operation_client_cmp ) ) { - c->c_n_ops_executing--; - } - CONNECTION_UNLOCK(c); - } - - if ( op->o_upstream ) { - Backend *b = NULL; - - c = op->o_upstream; - CONNECTION_LOCK(c); - if ( tavl_delete( &c->c_ops, op, operation_upstream_cmp ) ) { - c->c_n_ops_executing--; - b = (Backend *)c->c_private; - } - CONNECTION_UNLOCK(c); - - if ( b ) { - ldap_pvt_thread_mutex_lock( &b->b_mutex ); - b->b_n_ops_executing--; - ldap_pvt_thread_mutex_unlock( &b->b_mutex ); - } - } - ch_free( op ); } +/* + * See operation_destroy_from_client. + */ +void +operation_destroy_from_upstream( Operation *op ) +{ + Connection *client = op->o_client, *upstream = op->o_upstream; + Backend *b = NULL; + int race_state; + + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_upstream: " + "op=%p attempting to release operation\n", + op ); + + /* 1. liveness/refcnt adjustment and test */ + op->o_upstream_refcnt -= op->o_upstream_live; + op->o_upstream_live = 0; + + if ( op->o_upstream_refcnt ) { + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_upstream: " + "op=%p not dead yet\n", + op ); + return; + } + + /* 2. Remove from the operation map and adjust the pending op count */ + if ( tavl_delete( &upstream->c_ops, op, operation_upstream_cmp ) ) { + upstream->c_n_ops_executing--; + b = (Backend *)upstream->c_private; + } + + /* 3. Detect whether we entered a race to free op */ + ldap_pvt_thread_mutex_lock( &operation_mutex ); + race_state = op->o_freeing; + race_state |= SLAP_OP_FREEING_UPSTREAM; + ldap_pvt_thread_mutex_unlock( &operation_mutex ); + + /* 4. If we lost the race, deal with it */ + if ( race_state == SLAP_OP_FREEING_CLIENT ) { + /* + * We have raced to destroy op and the first one to lose on this side, + * leave a refcnt token on upstream so we don't destroy it before the + * other side has finished (it knows we did that when it examines + * o_freeing again). + */ + upstream->c_refcnt++; + } + CONNECTION_UNLOCK_INCREF(upstream); + + if ( b ) { + ldap_pvt_thread_mutex_lock( &b->b_mutex ); + b->b_n_ops_executing--; + ldap_pvt_thread_mutex_unlock( &b->b_mutex ); + } + if ( race_state ) { + CONNECTION_LOCK_DECREF(upstream); + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_upstream: " + "op=%p lost race, increased client refcnt\n", + op ); + return; + } + + CONNECTION_LOCK(client); + /* 5. If we raced the client side and won, reclaim the token */ + ldap_pvt_thread_mutex_lock( &operation_mutex ); + if ( op->o_freeing & SLAP_OP_FREEING_CLIENT ) { + /* + * We have raced to destroy op and won. To avoid freeing the connection + * under us, a refcnt token has been left over for us on the client, + * decref and see whether we are in charge of freeing it + */ + client->c_refcnt--; + } + ldap_pvt_thread_mutex_unlock( &operation_mutex ); + + /* 6. liveness/refcnt adjustment and test */ + op->o_client_refcnt -= op->o_client_live; + op->o_client_live = 0; + if ( op->o_client_refcnt ) { + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_upstream: " + "op=%p other side still alive, refcnt=%d\n", + op, op->o_client_refcnt ); + /* There must have been no race if op is still alive */ + ldap_pvt_thread_mutex_lock( &operation_mutex ); + op->o_freeing &= ~SLAP_OP_FREEING_UPSTREAM; + assert( op->o_freeing == 0 ); + ldap_pvt_thread_mutex_unlock( &operation_mutex ); + CONNECTION_UNLOCK(client); + CONNECTION_LOCK_DECREF(upstream); + return; + } + + /* 7. Remove from the operation map and TODO adjust the pending op count */ + tavl_delete( &client->c_ops, op, operation_client_cmp ); + CLIENT_UNLOCK_OR_DESTROY(client); + + /* 8. Release the operation */ + Debug( LDAP_DEBUG_TRACE, "operation_destroy_from_upstream: " + "op=%p destroyed operation from client connid=%lu, " + "client msgid=%d\n", + op, op->o_client_connid, op->o_client_msgid ); + ber_free( op->o_ber, 1 ); + ch_free( op ); + + CONNECTION_LOCK_DECREF(upstream); +} + /* * Entered holding c_mutex for now. */ @@ -158,6 +369,9 @@ operation_init( Connection *c, BerElement *ber ) op->o_client_connid = c->c_connid; op->o_ber = ber; + op->o_client_live = op->o_client_refcnt = 1; + op->o_upstream_live = op->o_upstream_refcnt = 1; + tag = ber_get_int( ber, &op->o_client_msgid ); if ( tag != LDAP_TAG_MSGID ) { goto fail; @@ -213,8 +427,8 @@ operation_abandon( Operation *op ) c = op->o_client; CONNECTION_LOCK(c); + operation_destroy_from_client( op ); CLIENT_UNLOCK_OR_DESTROY(c); - operation_destroy( op ); return; } @@ -260,8 +474,8 @@ operation_abandon( Operation *op ) CONNECTION_LOCK_DECREF(c); done: + operation_destroy_from_upstream( op ); UPSTREAM_UNLOCK_OR_DESTROY(c); - operation_destroy( op ); } int @@ -289,7 +503,7 @@ request_abandon( Connection *c, Operation *op ) rc = LDAP_SUCCESS; done: - operation_destroy( op ); + operation_destroy_from_client( op ); return rc; } @@ -311,8 +525,7 @@ operation_send_reject( CONNECTION_LOCK(c); found = ( tavl_delete( &c->c_ops, op, operation_client_cmp ) == op ); if ( !found && !send_anyway ) { - CONNECTION_UNLOCK(c); - return; + goto done; } CONNECTION_UNLOCK_INCREF(c); @@ -321,7 +534,9 @@ operation_send_reject( ber = c->c_pendingber; if ( ber == NULL && (ber = ber_alloc()) == NULL ) { ldap_pvt_thread_mutex_unlock( &c->c_io_mutex ); - CLIENT_LOCK_DESTROY(c); + CONNECTION_LOCK_DECREF(c); + operation_destroy_from_client( op ); + CLIENT_DESTROY(c); return; } c->c_pendingber = ber; @@ -334,9 +549,9 @@ operation_send_reject( client_write_cb( -1, 0, c ); - operation_destroy( op ); - CONNECTION_LOCK_DECREF(c); +done: + operation_destroy_from_client( op ); CLIENT_UNLOCK_OR_DESTROY(c); } @@ -345,7 +560,6 @@ operation_lost_upstream( Operation *op ) { operation_send_reject( op, LDAP_UNAVAILABLE, "connection to the remote server has been severed", 0 ); - operation_destroy( op ); } int @@ -356,6 +570,7 @@ request_process( Connection *client, Operation *op ) ber_int_t msgid; int rc = LDAP_SUCCESS; + op->o_client_refcnt++; CONNECTION_UNLOCK_INCREF(client); upstream = backend_select( op ); @@ -426,6 +641,9 @@ request_process( Connection *client, Operation *op ) UPSTREAM_UNLOCK_OR_DESTROY(upstream); CONNECTION_LOCK_DECREF(client); + if ( !--op->o_client_refcnt ) { + operation_destroy_from_client( op ); + } return rc; fail: @@ -436,5 +654,7 @@ fail: } operation_send_reject( op, LDAP_OTHER, "internal error", 0 ); CONNECTION_LOCK_DECREF(client); + op->o_client_refcnt--; + operation_destroy_from_client( op ); return rc; } diff --git a/servers/lloadd/proto-slap.h b/servers/lloadd/proto-slap.h index 3d9d1a40fc..273c4eb6a3 100644 --- a/servers/lloadd/proto-slap.h +++ b/servers/lloadd/proto-slap.h @@ -150,6 +150,7 @@ LDAP_SLAPD_V (const char *) slapd_slp_attrs; /* * operation.c */ +LDAP_SLAPD_V (ldap_pvt_thread_mutex_t) operation_mutex; LDAP_SLAPD_F (const char *) slap_msgtype2str( ber_tag_t tag ); LDAP_SLAPD_F (int) operation_upstream_cmp( const void *l, const void *r ); LDAP_SLAPD_F (int) operation_client_cmp( const void *l, const void *r ); @@ -157,7 +158,8 @@ LDAP_SLAPD_F (Operation *) operation_init( Connection *c, BerElement *ber ); LDAP_SLAPD_F (void) operation_abandon( Operation *op ); LDAP_SLAPD_F (void) operation_send_reject( Operation *op, int result, const char *msg, int send_anyway ); LDAP_SLAPD_F (void) operation_lost_upstream( Operation *op ); -LDAP_SLAPD_F (void) operation_destroy( Operation *op ); +LDAP_SLAPD_F (void) operation_destroy_from_client( Operation *op ); +LDAP_SLAPD_F (void) operation_destroy_from_upstream( Operation *op ); LDAP_SLAPD_F (int) request_abandon( Connection *c, Operation *op ); LDAP_SLAPD_F (int) request_process( Connection *c, Operation *op ); diff --git a/servers/lloadd/slap.h b/servers/lloadd/slap.h index ef7dd51507..3d7d208fe5 100644 --- a/servers/lloadd/slap.h +++ b/servers/lloadd/slap.h @@ -398,11 +398,24 @@ struct Connection { void *c_private; }; -struct Operation { - Connection *o_client, *o_upstream; - unsigned long o_client_connid, o_upstream_connid; +enum op_state { + SLAP_OP_NOT_FREEING = 0, + SLAP_OP_FREEING_UPSTREAM = 1 << 0, + SLAP_OP_FREEING_CLIENT = 1 << 1, +}; - ber_int_t o_client_msgid, o_upstream_msgid; +struct Operation { + Connection *o_client; + unsigned long o_client_connid; + int o_client_live, o_client_refcnt; + ber_int_t o_client_msgid; + + Connection *o_upstream; + unsigned long o_upstream_connid; + int o_upstream_live, o_upstream_refcnt; + ber_int_t o_upstream_msgid; + + enum op_state o_freeing; ber_tag_t o_tag; BerElement *o_ber; diff --git a/servers/lloadd/upstream.c b/servers/lloadd/upstream.c index a77d88a42b..9d24e61d98 100644 --- a/servers/lloadd/upstream.c +++ b/servers/lloadd/upstream.c @@ -75,7 +75,9 @@ forward_final_response( Operation *op, BerElement *ber ) "finishing up with request #%d for client %lu\n", op->o_client_msgid, op->o_client_connid ); rc = forward_response( op, ber ); - operation_destroy( op ); + CONNECTION_LOCK_DECREF(op->o_upstream); + operation_destroy_from_upstream( op ); + CONNECTION_UNLOCK_INCREF(op->o_upstream); return rc; } @@ -122,14 +124,21 @@ handle_bind_response( Operation *op, BerElement *ber ) ber_memfree( c->c_sasl_bind_mech.bv_val ); BER_BVZERO( &c->c_sasl_bind_mech ); } - CONNECTION_UNLOCK(c); + if ( rc ) { + CONNECTION_UNLOCK_INCREF(c); + } else { + CONNECTION_UNLOCK(c); + } break; } } done: if ( rc ) { - operation_destroy( op ); + CONNECTION_LOCK_DECREF(c); + operation_destroy_from_client( op ); + CLIENT_UNLOCK_OR_DESTROY(c); + ber_free( ber, 1 ); return rc; } @@ -179,7 +188,7 @@ handle_vc_bind_response( Operation *op, BerElement *ber ) tag = ber_scanf( ber, "o", &c->c_vc_cookie ); if ( tag == LBER_ERROR ) { rc = -1; - CONNECTION_UNLOCK(c); + CONNECTION_UNLOCK_INCREF(c); goto done; } tag = ber_peek_tag( ber, &len ); @@ -189,7 +198,7 @@ handle_vc_bind_response( Operation *op, BerElement *ber ) tag = ber_scanf( ber, "m", &creds ); if ( tag == LBER_ERROR ) { rc = -1; - CONNECTION_UNLOCK(c); + CONNECTION_UNLOCK_INCREF(c); goto done; } tag = ber_peek_tag( ber, &len ); @@ -199,7 +208,7 @@ handle_vc_bind_response( Operation *op, BerElement *ber ) tag = ber_scanf( ber, "m", &controls ); if ( tag == LBER_ERROR ) { rc = -1; - CONNECTION_UNLOCK(c); + CONNECTION_UNLOCK_INCREF(c); goto done; } } @@ -225,7 +234,7 @@ handle_vc_bind_response( Operation *op, BerElement *ber ) break; } } - CONNECTION_UNLOCK(c); + CONNECTION_UNLOCK_INCREF(c); ldap_pvt_thread_mutex_lock( &c->c_io_mutex ); output = c->c_pendingber; @@ -249,7 +258,9 @@ handle_vc_bind_response( Operation *op, BerElement *ber ) } done: - operation_destroy( op ); + CONNECTION_LOCK_DECREF(c); + operation_destroy_from_client( op ); + CLIENT_UNLOCK_OR_DESTROY(c); ber_free( ber, 1 ); return rc; } @@ -370,11 +381,16 @@ handle_one_response( Connection *c ) c->c_connid, slap_msgtype2str( tag ), needle.o_upstream_msgid ); } - CONNECTION_UNLOCK_INCREF(c); if ( handler ) { + op->o_upstream_refcnt++; + CONNECTION_UNLOCK_INCREF(c); rc = handler( op, ber ); + CONNECTION_LOCK_DECREF(c); + op->o_upstream_refcnt--; + if ( !op->o_upstream_live ) { + operation_destroy_from_upstream( op ); + } } - CONNECTION_LOCK_DECREF(c); fail: if ( rc ) { @@ -826,7 +842,6 @@ upstream_destroy( Connection *c ) "freeing connection %lu\n", c->c_connid ); - assert( c->c_state != SLAP_C_INVALID ); c->c_state = SLAP_C_INVALID; read_event = c->c_read_event; @@ -857,5 +872,19 @@ upstream_destroy( Connection *c ) CONNECTION_LOCK_DECREF(c); + /* + * If we attempted to destroy any operations, we might have lent a new + * refcnt token for a thread that raced us to that, let them call us again + * later + */ + assert( c->c_refcnt >= 0 ); + if ( c->c_refcnt ) { + c->c_state = SLAP_C_CLOSING; + Debug( LDAP_DEBUG_CONNS, "upstream_destroy: " + "connid=%lu aborting with refcnt=%d\n", + c->c_connid, c->c_refcnt ); + CONNECTION_UNLOCK(c); + return; + } connection_destroy( c ); }