Ticket 48769 - RFE: Be_txn extended operation plugin type

Bug Description:  In cases that plugins both use be_txn for pre or post
operation, and extended operations this can lead to deadlock. Additionally
plugin authors should not need to consider transactions in their code only
focusing on solving the issue.

Fix Description:  This adds a new plugin type, betxnextendedop which
automatically wraps extended operations in a transaction.

https://fedorahosted.org/389/ticket/48769

Author: wibrown

Review by: nhosoi (Thanks!)

ldap/servers/slapd/extendop.c

@@ -333,8 +333,63 @@ do_extended( Slapi_PBlock *pb )
 	slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_OID, extoid );
 	slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_VALUE, &extval );
 	slapi_pblock_set( pb, SLAPI_REQUESTOR_ISROOT, &pb->pb_op->o_isroot);
+
+    /* wibrown 201603 I want to rewrite this to get plugin p, and use that 
+     * rather than all these plugin_call_, that loop over the plugin lists
+     * We do "get plugin (oid).
+     * then we just hand *p into the call functions.
+     * much more efficient! :)
+     */
 	
-	rc = plugin_call_exop_plugins( pb, extoid );
+    slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c calling plugins ... \n");
+
+	rc = plugin_call_exop_plugins( pb, extoid, SLAPI_PLUGIN_EXTENDEDOP);
+
+    slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c called exop, got %d \n", rc);
+
+    if (rc == SLAPI_PLUGIN_EXTENDED_NOT_HANDLED) {
+        slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c calling betxn plugins ... \n");
+        /* Look up the correct backend to use. */
+        Slapi_Backend *be = plugin_extended_op_getbackend( pb, extoid );
+
+        if ( be == NULL ) {
+            slapi_log_error(SLAPI_LOG_FATAL, NULL, "extendop.c plugin_extended_op_getbackend was unable to retrieve a backend!!!\n");
+            rc = SLAPI_PLUGIN_EXTENDED_NO_BACKEND_AVAILABLE;
+        } else {
+            /* We need to make a new be pb here because when you set SLAPI_BACKEND
+             * you overwrite the plg parts of the pb. So if we re-use pb
+             * you actually nuke the request, and everything hangs. (╯°□°)╯︵ ┻━┻
+             */
+            Slapi_PBlock *be_pb = NULL;
+            be_pb = slapi_pblock_new();
+            slapi_pblock_set(be_pb, SLAPI_BACKEND, be);
+
+            int txn_rc = slapi_back_transaction_begin(be_pb);
+            if (txn_rc) {
+                slapi_log_error(SLAPI_LOG_FATAL, NULL, "exendop.c Failed to start be_txn for plugin_call_exop_plugins %d\n", txn_rc);
+            } else {
+                rc = plugin_call_exop_plugins( pb, extoid, SLAPI_PLUGIN_BETXNEXTENDEDOP);
+                slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c called betxn exop, got %d \n", rc);
+                if (rc == LDAP_SUCCESS || rc == SLAPI_PLUGIN_EXTENDED_SENT_RESULT) {
+                    /* commit */
+                    txn_rc = slapi_back_transaction_commit(be_pb);
+                    if (txn_rc == 0) {
+                        slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c commit with result %d \n", txn_rc);
+                    } else {
+                        slapi_log_error(SLAPI_LOG_FATAL, NULL, "extendop.c Unable to commit commit with result %d \n", txn_rc);
+                    }
+                } else {
+                    /* abort */
+                    txn_rc = slapi_back_transaction_abort(be_pb);
+                    slapi_log_error(SLAPI_LOG_FATAL, NULL, "extendop.c abort with result %d \n", txn_rc);
+                }
+            } /* txn_rc */
+            if (be_pb != NULL) {
+                slapi_pblock_destroy(be_pb); /* Clean up after ourselves */
+            }
+            slapi_log_error(SLAPI_LOG_TRACE, NULL, "exendop.c plugin_call_exop_plugins rc final %d\n", rc);
+        } /* if be */
+    }
 
 	if ( SLAPI_PLUGIN_EXTENDED_SENT_RESULT != rc ) {
 		if ( SLAPI_PLUGIN_EXTENDED_NOT_HANDLED == rc ) {

ldap/servers/slapd/pblock.c

@@ -727,23 +727,33 @@ slapi_pblock_get( Slapi_PBlock *pblock, int arg, void *value )
 
 	/* extendedop plugin functions */
 	case SLAPI_PLUGIN_EXT_OP_FN:
-		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP ) {
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP &&
+             pblock->pb_plugin->plg_type != SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
 			return( -1 );
 		}
 		(*(IFP *)value) = pblock->pb_plugin->plg_exhandler;
 		break;
 	case SLAPI_PLUGIN_EXT_OP_OIDLIST:
-		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP ) {
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP &&
+             pblock->pb_plugin->plg_type != SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
 			return( -1 );
 		}
 		(*(char ***)value) = pblock->pb_plugin->plg_exoids;
 		break;
 	case SLAPI_PLUGIN_EXT_OP_NAMELIST:
-		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP ) {
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP &&
+             pblock->pb_plugin->plg_type != SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
 			return( -1 );
 		}
 		(*(char ***)value) = pblock->pb_plugin->plg_exnames;
 		break;
+	case SLAPI_PLUGIN_EXT_OP_BACKEND_FN:
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP &&
+             pblock->pb_plugin->plg_type != SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
+			return( -1 );
+		}
+		(*(IFP *)value) = pblock->pb_plugin->plg_be_exhandler;
+		break;
 
 	/* preoperation plugin functions */
 	case SLAPI_PLUGIN_PRE_BIND_FN:
@@ -2353,24 +2363,34 @@ slapi_pblock_set( Slapi_PBlock *pblock, int arg, void *value )
 
 	/* extendedop plugin functions */
 	case SLAPI_PLUGIN_EXT_OP_FN:
-		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP ) {
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP &&
+             pblock->pb_plugin->plg_type != SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
 			return( -1 );
 		}
 		pblock->pb_plugin->plg_exhandler = (IFP) value;
 		break;
 	case SLAPI_PLUGIN_EXT_OP_OIDLIST:
-		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP ) {
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP &&
+             pblock->pb_plugin->plg_type != SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
 			return( -1 );
 		}
 		pblock->pb_plugin->plg_exoids = (char **) value;
 		ldapi_register_extended_op( (char **)value );
 		break;
 	case SLAPI_PLUGIN_EXT_OP_NAMELIST:
-		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP ) {
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP &&
+             pblock->pb_plugin->plg_type != SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
 			return( -1 );
 		}
 		pblock->pb_plugin->plg_exnames = (char **) value;
 		break;
+	case SLAPI_PLUGIN_EXT_OP_BACKEND_FN:
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_EXTENDEDOP &&
+             pblock->pb_plugin->plg_type != SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
+			return( -1 );
+		}
+		pblock->pb_plugin->plg_be_exhandler = (IFP) value;
+		break;
 
 	/* preoperation plugin functions */
 	case SLAPI_PLUGIN_PRE_BIND_FN:

ldap/servers/slapd/plugin.c

@@ -485,44 +485,54 @@ plugin_call_entryfetch_plugins(char **entrystr, uint *size)
  *	returned by the plugins we called).
  */
 int
-plugin_call_exop_plugins( Slapi_PBlock *pb, char *oid )
+plugin_call_exop_plugins( Slapi_PBlock *pb, char *oid, int whichtype )
 {
-	struct slapdplugin	*p;
-	int			i, rc;
-	int lderr = SLAPI_PLUGIN_EXTENDED_NOT_HANDLED;
-
-	for ( p = global_plugin_list[PLUGIN_LIST_EXTENDED_OPERATION]; p != NULL; p = p->plg_next ) {
-		if ( p->plg_exhandler != NULL ) {
-			if ( p->plg_exoids != NULL ) {
-				for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
-					if ( strcasecmp( oid, p->plg_exoids[i] )
-					    == 0 ) {
-						break;
-					}
-				}
-				if (  p->plg_exoids[i] == NULL ) {
-					continue;
-				}
-			}
+    struct slapdplugin  *p;
+    int         i, rc;
+    int list_type;
+    int lderr = SLAPI_PLUGIN_EXTENDED_NOT_HANDLED;
+
+    if (whichtype == SLAPI_PLUGIN_EXTENDEDOP) {
+        list_type = PLUGIN_LIST_EXTENDED_OPERATION;
+    } else if (whichtype == SLAPI_PLUGIN_BETXNEXTENDEDOP) {
+        list_type = PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION;
+    } else {
+        slapi_log_error(SLAPI_LOG_FATAL, NULL, "plugin_call_exop_plugins unknown plugin list type %d\n", whichtype);
+        return( lderr );
+    }
 
-			slapi_pblock_set( pb, SLAPI_PLUGIN, p );
-			set_db_default_result_handlers( pb );
-			if ( (rc = (*p->plg_exhandler)( pb ))
-			    == SLAPI_PLUGIN_EXTENDED_SENT_RESULT ) {
-				return( rc );	/* result sent */
-			} else if ( rc != SLAPI_PLUGIN_EXTENDED_NOT_HANDLED ) {
-				/*
-				 * simple merge: report last real error
-				 */
-				if ( lderr == SLAPI_PLUGIN_EXTENDED_NOT_HANDLED
-				    || rc != LDAP_SUCCESS ) {
-					lderr = rc;
-				}
-			}
-		}
-	}	
+    for ( p = global_plugin_list[list_type]; p != NULL; p = p->plg_next ) {
+        if ( p->plg_exhandler != NULL && p->plg_type == whichtype ) {
+            if ( p->plg_exoids != NULL ) {
+                for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
+                    if ( strcasecmp( oid, p->plg_exoids[i] )
+                        == 0 ) {
+                        break;
+                    }
+                }
+                if (  p->plg_exoids[i] == NULL ) {
+                    continue;
+                }
+            }
+
+            slapi_pblock_set( pb, SLAPI_PLUGIN, p );
+            set_db_default_result_handlers( pb );
+            if ( (rc = (*p->plg_exhandler)( pb ))
+                == SLAPI_PLUGIN_EXTENDED_SENT_RESULT ) {
+                return( rc );   /* result sent */
+            } else if ( rc != SLAPI_PLUGIN_EXTENDED_NOT_HANDLED ) {
+                /*
+                 * simple merge: report last real error
+                 */
+                if ( lderr == SLAPI_PLUGIN_EXTENDED_NOT_HANDLED
+                    || rc != LDAP_SUCCESS ) {
+                    lderr = rc;
+                }
+            }
+        }
+    }
 
-	return( lderr );
+    return( lderr );
 }
 
 
@@ -539,36 +549,77 @@ plugin_call_exop_plugins( Slapi_PBlock *pb, char *oid )
 const char *
 plugin_extended_op_oid2string( const char *oid )
 {
-	struct slapdplugin	*p;
-	int					i, j;
-	const char			*rval = NULL;
-
-	for ( p = global_plugin_list[PLUGIN_LIST_EXTENDED_OPERATION]; p != NULL;
-			p = p->plg_next ) {
-		if ( p->plg_exhandler != NULL && p->plg_exoids != NULL ) {
-			for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
-				if ( strcasecmp( oid, p->plg_exoids[i] ) == 0 ) {
-					if ( NULL != p->plg_exnames ) {
-						for ( j = 0; j < i && p->plg_exnames[j] != NULL; ++j ) {
-							;
-						}
-						rval = p->plg_exnames[j];		/* OID-related name */
-					}
+    struct slapdplugin  *p;
+    int                 i, j, l, list_type;
+    const char          *rval = NULL;
+    int list_types[] = {PLUGIN_LIST_EXTENDED_OPERATION, PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION};
+
+    /* I feel there may be a better way to achieve this, but it works. */
+    for ( l = 0; l < 2; ++l ) {
+        list_type = list_types[l];
+        for ( p = global_plugin_list[list_type]; p != NULL; p = p->plg_next ) {
+            if ( p->plg_exhandler != NULL && p->plg_exoids != NULL ) {
+                for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
+                    if ( strcasecmp( oid, p->plg_exoids[i] ) == 0 ) {
+                        if ( NULL != p->plg_exnames ) {
+                            for ( j = 0; j < i && p->plg_exnames[j] != NULL; ++j ) {
+                                ;
+                            }
+                            rval = p->plg_exnames[j];       /* OID-related name */
+                        }
+
+                        if ( NULL == rval ) {
+                            if ( NULL != p->plg_desc.spd_id ) {
+                                rval = p->plg_desc.spd_id;  /* short name */
+                            } else {
+                                rval = p->plg_name;         /* RDN */
+                            }
+                        }
+                        break;
+                    }
+                } /* for */
+            } /* If */
+        } /* for p in global_plugin list */
+    } /* list type */
 
-					if ( NULL == rval ) {
-						if ( NULL != p->plg_desc.spd_id ) {
-							rval = p->plg_desc.spd_id;	/* short name */
-						} else {
-							rval = p->plg_name;			/* RDN */
-						}
-					}
-					break;
-				}
-			}
-		}
-	}
+    return( rval );
+}
+
+
+Slapi_Backend *
+plugin_extended_op_getbackend( Slapi_PBlock *pb, char *oid )
+{
+    struct slapdplugin  *p;
+    int i;
+    int rc;
+    /* This could be an error type, but for now we expect the caller to check 
+     * that it's not null
+     */
+    Slapi_Backend *result = NULL;
+
+    for ( p = global_plugin_list[PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION]; p != NULL; p = p->plg_next ) {
+        if ( p->plg_be_exhandler != NULL && p->plg_type == SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
+            if ( p->plg_exoids != NULL ) {
+                for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
+                    if ( strcasecmp( oid, p->plg_exoids[i] ) == 0 ) {
+                        break;
+                    }
+                }
+                if (  p->plg_exoids[i] == NULL ) {
+                    continue;
+                }
+            }
+
+            rc = (*p->plg_be_exhandler)( pb, &result );
+            if (rc != LDAP_SUCCESS) {
+                /* Do we need to do anything? Or it is the parents job? */
+                result = NULL;
+            }
+            break;
+        }
+    }
 
-	return( rval );
+    return( result );
 }
 
 static int
@@ -2264,6 +2315,9 @@ plugin_get_type_and_list(
 	} else if ( strcasecmp( plugintype, "index" ) == 0 ) {
         *type = SLAPI_PLUGIN_INDEX;
         plugin_list_index= PLUGIN_LIST_INDEX;
+	} else if ( strcasecmp( plugintype, "betxnextendedop" ) == 0 ) {
+		*type = SLAPI_PLUGIN_BETXNEXTENDEDOP;
+    	plugin_list_index= PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION;
 	} else {
         return( 1 ); /* unknown plugin type - pass to backend */
 	}

ldap/servers/slapd/proto-slap.h

@@ -877,7 +877,8 @@ void global_plugin_init();
 int plugin_call_plugins( Slapi_PBlock *, int );
 int plugin_setup(Slapi_Entry *plugin_entry, struct slapi_componentid *group,
 	slapi_plugin_init_fnptr initfunc, int add_to_dit, char *returntext);
-int plugin_call_exop_plugins( Slapi_PBlock *pb, char *oid );
+int plugin_call_exop_plugins( Slapi_PBlock *pb, char *oid, int whichtype );
+Slapi_Backend * plugin_extended_op_getbackend( Slapi_PBlock *pb, char *oid);
 const char *plugin_extended_op_oid2string( const char *oid );
 void plugin_closeall(int close_backends, int close_globals);
 void plugin_startall(int argc, char **argv, char **plugin_list);

ldap/servers/slapd/slap.h

@@ -681,22 +681,23 @@ struct matchingRuleList {
 #define PLUGIN_LIST_INTERNAL_PREOPERATION 5
 #define PLUGIN_LIST_INTERNAL_POSTOPERATION 6
 #define PLUGIN_LIST_EXTENDED_OPERATION 7
-#define PLUGIN_LIST_BACKEND_MAX 8
+#define PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION 8
+#define PLUGIN_LIST_BACKEND_MAX 9
 
 /* Global Plugins */
-#define PLUGIN_LIST_ACL 9
-#define PLUGIN_LIST_MATCHINGRULE 10
-#define PLUGIN_LIST_SYNTAX 11
-#define PLUGIN_LIST_ENTRY 12
-#define PLUGIN_LIST_OBJECT 13
-#define PLUGIN_LIST_PWD_STORAGE_SCHEME 14
-#define PLUGIN_LIST_VATTR_SP 15	/* DBDB */
-#define PLUGIN_LIST_REVER_PWD_STORAGE_SCHEME 16
-#define PLUGIN_LIST_LDBM_ENTRY_FETCH_STORE 17
-#define PLUGIN_LIST_INDEX 18
-#define PLUGIN_LIST_BETXNPREOPERATION 19
-#define PLUGIN_LIST_BETXNPOSTOPERATION 20
-#define PLUGIN_LIST_GLOBAL_MAX 21
+#define PLUGIN_LIST_ACL 10
+#define PLUGIN_LIST_MATCHINGRULE 11
+#define PLUGIN_LIST_SYNTAX 12
+#define PLUGIN_LIST_ENTRY 13
+#define PLUGIN_LIST_OBJECT 14
+#define PLUGIN_LIST_PWD_STORAGE_SCHEME 15
+#define PLUGIN_LIST_VATTR_SP 16	/* DBDB */
+#define PLUGIN_LIST_REVER_PWD_STORAGE_SCHEME 17
+#define PLUGIN_LIST_LDBM_ENTRY_FETCH_STORE 18
+#define PLUGIN_LIST_INDEX 19
+#define PLUGIN_LIST_BETXNPREOPERATION 20
+#define PLUGIN_LIST_BETXNPOSTOPERATION 21
+#define PLUGIN_LIST_GLOBAL_MAX 22
 
 /* plugin configuration attributes */
 #define ATTR_PLUGIN_PATH				"nsslapd-pluginPath"
@@ -900,10 +901,12 @@ struct slapdplugin {
 			char	**plg_un_pe_exoids;	  /* exop oids */
 			char	**plg_un_pe_exnames;  /* exop names (may be NULL) */
 			IFP	plg_un_pe_exhandler;	  /* handler */
+			IFP	plg_un_pe_be_exhandler;	  /* handler to retrieve the be name for the operation */
 		} plg_un_pe;
 #define plg_exoids		plg_un.plg_un_pe.plg_un_pe_exoids
 #define plg_exnames		plg_un.plg_un_pe.plg_un_pe_exnames
 #define plg_exhandler		plg_un.plg_un_pe.plg_un_pe_exhandler
+#define plg_be_exhandler		plg_un.plg_un_pe.plg_un_pe_be_exhandler
 
 
 		/* pre-operation plugin structure */

ldap/servers/slapd/slapi-plugin.h

@@ -6745,6 +6745,7 @@ time_t slapi_current_time( void );
 #define SLAPI_PLUGIN_INDEX			18
 #define	SLAPI_PLUGIN_BETXNPREOPERATION		19
 #define SLAPI_PLUGIN_BETXNPOSTOPERATION		20
+#define SLAPI_PLUGIN_BETXNEXTENDEDOP			21
 
 /*
  * special return values for extended operation plugins (zero or positive
@@ -6752,6 +6753,7 @@ time_t slapi_current_time( void );
  */
 #define SLAPI_PLUGIN_EXTENDED_SENT_RESULT	-1
 #define SLAPI_PLUGIN_EXTENDED_NOT_HANDLED	-2
+#define SLAPI_PLUGIN_EXTENDED_NO_BACKEND_AVAILABLE -3
 
 /*
  * Return values of plugins:
@@ -6876,6 +6878,7 @@ typedef struct slapi_plugindesc {
 #define SLAPI_PLUGIN_EXT_OP_FN			300
 #define SLAPI_PLUGIN_EXT_OP_OIDLIST		301
 #define SLAPI_PLUGIN_EXT_OP_NAMELIST	302
+#define SLAPI_PLUGIN_EXT_OP_BACKEND_FN  1948
 
 /* preoperation plugin functions */
 #define SLAPI_PLUGIN_PRE_BIND_FN		401