From acba4f77556d50e2cb6cfb75dd96fa2ea32a2452 Mon Sep 17 00:00:00 2001 From: Hallvard Furuseth Date: Thu, 21 May 2009 23:22:46 +0000 Subject: [PATCH] For ITS#6104: Protect initial o_abandon/o_cancel with op->o_conn->c_mutex. --- servers/slapd/abandon.c | 37 +++++++++++++++++++++---------------- servers/slapd/cancel.c | 26 ++++++++++++++------------ 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/servers/slapd/abandon.c b/servers/slapd/abandon.c index 9236b9043e..8e08707756 100644 --- a/servers/slapd/abandon.c +++ b/servers/slapd/abandon.c @@ -36,6 +36,7 @@ do_abandon( Operation *op, SlapReply *rs ) { ber_int_t id; Operation *o; + const char *msg; Debug( LDAP_DEBUG_TRACE, "%s do_abandon\n", op->o_log_prefix, 0, 0 ); @@ -72,28 +73,21 @@ do_abandon( Operation *op, SlapReply *rs ) } ldap_pvt_thread_mutex_lock( &op->o_conn->c_mutex ); - /* - * find the operation being abandoned and set the o_abandon - * flag. It's up to the backend to periodically check this - * flag and abort the operation at a convenient time. - */ + /* Find the operation being abandoned. */ LDAP_STAILQ_FOREACH( o, &op->o_conn->c_ops, o_next ) { if ( o->o_msgid == id ) { - o->o_abandon = 1; break; } } - if ( o ) { - op->orn_msgid = id; - - op->o_bd = frontendDB; - rs->sr_err = frontendDB->be_abandon( op, rs ); - - } else { + if ( o == NULL ) { + msg = "not found"; + /* The operation is not active. Just discard it if found. */ LDAP_STAILQ_FOREACH( o, &op->o_conn->c_pending_ops, o_next ) { if ( o->o_msgid == id ) { + msg = "discarded"; + /* FIXME: This traverses c_pending_ops yet again. */ LDAP_STAILQ_REMOVE( &op->o_conn->c_pending_ops, o, Operation, o_next ); LDAP_STAILQ_NEXT(o, o_next) = NULL; @@ -102,13 +96,24 @@ do_abandon( Operation *op, SlapReply *rs ) break; } } + } else { + msg = "found"; + /* Set the o_abandon flag in the to-be-abandoned operation. + * The backend can periodically check this flag and abort the + * operation at a convenient time. However it should "send" + * the response anyway, with result code SLAPD_ABANDON. + * The functions in result.c will intercept the message. + */ + o->o_abandon = 1; + op->orn_msgid = id; + op->o_bd = frontendDB; + rs->sr_err = frontendDB->be_abandon( op, rs ); } ldap_pvt_thread_mutex_unlock( &op->o_conn->c_mutex ); - Debug( LDAP_DEBUG_TRACE, "%s do_abandon: op=%ld %sfound\n", - op->o_log_prefix, - (long) id, o ? "" : "not " ); + Debug( LDAP_DEBUG_TRACE, "%s do_abandon: op=%ld %s\n", + op->o_log_prefix, (long) id, msg ); return rs->sr_err; } diff --git a/servers/slapd/cancel.c b/servers/slapd/cancel.c index ce0b1fdbc0..ef35046c0a 100644 --- a/servers/slapd/cancel.c +++ b/servers/slapd/cancel.c @@ -65,6 +65,7 @@ int cancel_extop( Operation *op, SlapReply *rs ) } ldap_pvt_thread_mutex_lock( &op->o_conn->c_mutex ); + LDAP_STAILQ_FOREACH( o, &op->o_conn->c_pending_ops, o_next ) { if ( o->o_msgid == opid ) { LDAP_STAILQ_REMOVE( &op->o_conn->c_pending_ops, o, Operation, o_next ); @@ -78,21 +79,25 @@ int cancel_extop( Operation *op, SlapReply *rs ) LDAP_STAILQ_FOREACH( o, &op->o_conn->c_ops, o_next ) { if ( o->o_msgid == opid ) { - o->o_abandon = 1; break; } } - ldap_pvt_thread_mutex_unlock( &op->o_conn->c_mutex ); - - if ( o ) { - if ( o->o_cancel != SLAP_CANCEL_NONE ) { - rs->sr_text = "message ID already being cancelled"; - return LDAP_PROTOCOL_ERROR; - } - + if ( o == NULL ) { + rc = LDAP_NO_SUCH_OPERATION; + rs->sr_text = "message ID not found"; + } else if ( o->o_cancel != SLAP_CANCEL_NONE ) { + rc = LDAP_PROTOCOL_ERROR; + rs->sr_text = "message ID already being cancelled"; + } else { + rc = LDAP_SUCCESS; o->o_cancel = SLAP_CANCEL_REQ; + o->o_abandon = 1; + } + + ldap_pvt_thread_mutex_unlock( &op->o_conn->c_mutex ); + if ( rc == LDAP_SUCCESS ) { LDAP_STAILQ_FOREACH( op->o_bd, &backendDB, be_next ) { if( !op->o_bd->be_cancel ) continue; @@ -113,9 +118,6 @@ int cancel_extop( Operation *op, SlapReply *rs ) } o->o_cancel = SLAP_CANCEL_DONE; - } else { - rs->sr_text = "message ID not found"; - rc = LDAP_NO_SUCH_OPERATION; } return rc; -- 2.39.5