ITS#6104

diff --git a/CHANGES b/CHANGES
index 748bf419ca7c786c8f4885d8c2dc11f34fb41d12..af091212dfc3535cd198618f6ebf8a14faa714ea 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -23,6 +23,7 @@ OpenLDAP 2.4.17 Engineering
        Fixed slapd sending cancelled operations results (ITS#6103)
        Fixed slapd slapi_entry_has_children (ITS#6132)
        Fixed slapd sockets usage on windows (ITS#6039)
+       Fixed slapd some abandon and cancel race conditions (ITS#6104)
        Fixed slapd tls context after changes (ITS#6135)
        Fixed slapd-hdb freeing of already freed entries (ITS#6074)
        Fixed slapd-hdb entryinfo cleanup (ITS#6088)

diff --git a/servers/slapd/abandon.c b/servers/slapd/abandon.c
index 9236b9043e077090062ab2d967a9cff3ebf0d36b..8e0870775685c6b739dc0b0c9f0704edac6d1600 100644 (file)
--- 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 ce0b1fdbc06c5bbf7d9d994b34df8e8d56d25b32..ef35046c0af611c4f39c6156087fd42ebc911989 100644 (file)
--- 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;