Remove useless module flags

src/lib/server/module.h

@@ -47,12 +47,9 @@ typedef struct module_list_s			module_list_t;
 
 DIAG_OFF(attributes)
 typedef enum CC_HINT(flag_enum) {
-	MODULE_TYPE_THREAD_SAFE	= (0 << 0), 	//!< Module is threadsafe.
-	MODULE_TYPE_THREAD_UNSAFE = (1 << 0), 	//!< Module is not threadsafe.
-						//!< Server will protect calls with mutex.
-	MODULE_TYPE_RESUMABLE	= (1 << 2), 	//!< does yield / resume
-
-	MODULE_TYPE_RETRY 	= (1 << 3) 	//!< can handle retries
+	MODULE_TYPE_THREAD_UNSAFE	= (1 << 0), 	//!< Module is not threadsafe.
+							//!< Server will protect calls with mutex.
+	MODULE_TYPE_RETRY		= (1 << 3) 	//!< can handle retries
 } module_flags_t;
 DIAG_ON(attributes)
 
@@ -160,15 +157,32 @@ struct module_s {
 
 	conf_parser_t const		*config;		//!< How to convert a CONF_SECTION to a module instance.
 
+	size_t				boot_size;		//!< Size of the module's bootstrap data.
+	char const			*boot_type;		//!< talloc type to assign to bootstrap data.
+
 	size_t				inst_size;		//!< Size of the module's instance data.
 	char const			*inst_type;		//!< talloc type to assign to instance data.
 
 	module_instantiate_t		bootstrap;		//!< Callback to allow the module to register any global
 								///< resources like xlat functions and attributes.
+								///< Instance data is read only during the bootstrap phase
+								///< and MUST NOT be modified.
+								///< Any attributes added during this phase that the module
+								///< need to be re-resolved during the instantiation phase
+								///< so that dynamic modules (which don't run bootstrap)
+								///< work correctly.
+								///< @note Not modifying the instance data is not just a
+								///< suggestion, if you try, you'll generate a SIGBUS
+								///< or SIGSEGV and it won't be obvious why.
+
 	module_instantiate_t		instantiate;		//!< Callback to allow the module to register any
 								///< per-instance resources like sockets and file handles.
-	module_detach_t			detach;			//!< Clean up module resources from both the bootstrap
-								///< and instantiation pahses.
+								///< After instantiate completes the module instance data
+								///< is mprotected to prevent modification.
+
+	module_detach_t			detach;			//!< Clean up module resources from the instantiation pahses.
+
+	module_detach_t			unstrap;		//!< Clean up module resources from both the bootstrap phase.
 
 	module_flags_t			flags;			//!< Flags that control how a module starts up and how
 								///< a module is called.
@@ -191,33 +205,33 @@ typedef enum CC_HINT(flag_enum) {
 								///< yet instantiated.
 	MODULE_INSTANCE_INSTANTIATED		= (1 << 2),	//!< Module instance has been bootstrapped and
 								///< instantiated.
-	MODULE_INSTANCE_NO_THREAD_INSTANCE	= (1 << 3)	//!< Not set internally, but can be used to prevent
+	MODULE_INSTANCE_NO_THREAD_INSTANTIATE	= (1 << 3)	//!< Not set internally, but can be used to prevent
 								///< thread instantiation for certain modules.
 } module_instance_state_t;
 DIAG_ON(attributes)
 
+typedef struct {
+	TALLOC_CTX 			*ctx;		//!< ctx data is allocated in.
+	void				*start;		//!< Start address which may be passed to mprotect.
+	size_t 				len;		//!< How much data we need mprotect to protect.
+} module_data_pool_t;
+
 /** Per instance data
  *
  * Per-instance data structure, to correlate the modules with the
  * instance names (may NOT be the module names!), and the per-instance
  * data structures.
  */
 struct module_instance_s {
-	module_instance_state_t		state;		//!< What's been done with this module so far.
+       /** @name Fields that are most frequently accessed at runtime
+	*
+	* Putting them first gives us the greatest chance of the pointers being prefetched.
+	* @{
+	*/
+	void				*data;		//!< Module's instance data.  This is most
+							///< frequently access comes first.
 
-	fr_rb_node_t			name_node;	//!< Entry in the name tree.
-	fr_rb_node_t			data_node;	//!< Entry in the data tree.
-
-	module_list_t			*ml;		//!< Module list this instance belongs to.
-
-	char const			*name;		//!< Instance name e.g. user_database.
-
-	dl_module_t			*module;	//!< dynamic loader handle.  Contains the module's
-							///< dlhandle, and the functions it exports.
-							///< The dl_module is reference counted so that it
-							///< can be freed automatically when the last instance
-							///< is freed.  This will also (usually) unload the
-							///< .so or .dylib.
+	void				*boot;		//!< Data allocated during the boostrap phase
 
 	module_t const			*exported;	//!< Public module structure.  Cached for convenience.
 							///< This exports module methods, i.e. the functions
@@ -226,16 +240,16 @@ struct module_instance_s {
 							///< but with a different type, containing additional
 							///< instance callbacks to make it easier to use.
 
-	void				*data;		//!< Module's instance data.
-	CONF_SECTION			*conf;		//!< Module's instance configuration.
-
-	module_instance_t const		*parent;	//!< Parent module's instance (if any).
-
 	pthread_mutex_t			mutex;		//!< Used prevent multiple threads entering a thread
 							///< unsafe module simultaneously.
 
-	uint32_t			number;		//!< Unique module number.  Used to assign a stable
-							///< number to each module instance.
+	dl_module_t			*module;	//!< dynamic loader handle.  Contains the module's
+							///< dlhandle, and the functions it exports.
+							///< The dl_module is reference counted so that it
+							///< can be freed automatically when the last instance
+							///< is freed.  This will also (usually) unload the
+							///< .so or .dylib.
+	/** @} */
 
 	/** @name Return code overrides
 	 * @{
@@ -247,10 +261,42 @@ struct module_instance_s {
 							//!< has been set to true.
 
 	unlang_actions_t       		actions;	//!< default actions and retries.
+	/** @} */
 
+       /** @name Allow module instance data to be resolved by name or data, and to get back to the module list
+	* @{
+	*/
+	module_list_t			*ml;		//!< Module list this instance belongs to.
+	fr_rb_node_t			name_node;	//!< Entry in the name tree.
+	fr_rb_node_t			data_node;	//!< Entry in the data tree.
+	uint32_t			number;		//!< Unique module number.  Used to assign a stable
+							///< number to each module instance.
+	/** @} */
+
+       /** @name These structures allow mprotect to protect/unprotest bootstrap and instance data
+	* @{
+	*/
+	module_data_pool_t		inst_pool;	//!< Data to allow mprotect state toggling
+							///< for instance data.
+	module_data_pool_t		boot_pool;	//!< Data to allow mprotect state toggling
+							///< for bootstrap data.
 	/** @} */
 
-	bool				detached;	//!< Whether the detach function has been called.
+       /** @name Module instance state
+	* @{
+	*/
+	module_instance_state_t		mask;		//!< Prevent phases from being executed.
+	module_instance_state_t		state;		//!< What's been done with this module so far.
+	CONF_SECTION			*conf;		//!< Module's instance configuration.
+	/** @} */
+
+       /** @name Misc fields
+	* @{
+	*/
+	char const			*name;		//!< Instance name e.g. user_database.
+
+	module_instance_t const		*parent;	//!< Parent module's instance (if any).
+	/** @} */
 };
 
 /** Per thread per instance data
@@ -272,6 +318,42 @@ struct module_thread_instance_s {
 	uint64_t			active_callers; //! number of active callers.  i.e. number of current yields
 };
 
+/** Should we bootstrap this module instance?
+ *
+ * @param[in] mi	to check.
+ * @return
+ *	- true if the module instance should be bootstrapped.
+ *	- false if the module instance has already been bootstrapped.
+ */
+static inline bool module_instance_skip_bootstrap(module_instance_t *mi)
+{
+	return ((mi->state | mi->mask) & MODULE_INSTANCE_BOOTSTRAPPED);
+}
+
+/** Should we instantiate this module instance?
+ *
+ * @param[in] mi	to check.
+ * @return
+ *	- true if the module instance should be instantiated.
+ *	- false if the module instance has already been instantiated.
+ */
+static inline bool module_instance_skip_instantiate(module_instance_t *mi)
+{
+	return ((mi->state | mi->mask) & MODULE_INSTANCE_INSTANTIATED);
+}
+
+/** Should we instantiate this module instance in a new thread?
+ *
+ * @param[in] mi	to check.
+ * @return
+ *	- true if the module instance should be instantiated in a new thread.
+ *	- false if the module instance has already been instantiated in a new thread.
+ */
+static inline bool module_instance_skip_thread_instantiate(module_instance_t *mi)
+{
+	return ((mi->state | mi->mask) & MODULE_INSTANCE_NO_THREAD_INSTANTIATE);
+}
+
 /** Callback to retrieve thread-local data for a module
  *
  * @param[in] mi	to add data to (use mi->ml for the module list).

src/modules/rlm_cipher/rlm_cipher.c

@@ -1375,7 +1375,6 @@ module_rlm_t rlm_cipher = {
 	.common = {
 		.magic			= MODULE_MAGIC_INIT,
 		.name			= "cipher",
-		.flags			= MODULE_TYPE_THREAD_SAFE,
 		.inst_size		= sizeof(rlm_cipher_t),
 		.thread_inst_size	= sizeof(rlm_cipher_rsa_thread_inst_t),
 		.config			= module_config,

src/modules/rlm_client/rlm_client.c

@@ -381,7 +381,6 @@ module_rlm_t rlm_client = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "dynamic_clients",
-		.flags		= MODULE_TYPE_THREAD_SAFE,		/* type */
 		.onload		= mod_load,
 		.unload		= mod_unload
 	},

src/modules/rlm_couchbase/rlm_couchbase.c

@@ -549,7 +549,6 @@ module_rlm_t rlm_couchbase = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "couchbase",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_couchbase_t),
 		.config		= module_config,
 		.onload		= mod_load,

src/modules/rlm_exec/rlm_exec.c

@@ -521,7 +521,6 @@ module_rlm_t rlm_exec = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "exec",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_exec_t),
 		.config		= module_config,
 		.bootstrap	= mod_bootstrap,

src/modules/rlm_icmp/rlm_icmp.c

@@ -550,7 +550,6 @@ module_rlm_t rlm_icmp = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "icmp",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_icmp_t),
 		.config		= module_config,
 		.bootstrap	= mod_bootstrap,

src/modules/rlm_idn/rlm_idn.c

@@ -165,7 +165,6 @@ module_rlm_t rlm_idn = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "idn",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_idn_t),
 		.config		= mod_config,
 		.bootstrap	= mod_bootstrap

src/modules/rlm_imap/rlm_imap.c

@@ -286,7 +286,6 @@ module_rlm_t rlm_imap = {
 	.common = {
 		.magic		        = MODULE_MAGIC_INIT,
 		.name		        = "imap",
-		.flags		        = MODULE_TYPE_THREAD_SAFE,
 		.inst_size	        = sizeof(rlm_imap_t),
 		.thread_inst_size   	= sizeof(rlm_imap_thread_t),
 		.config		        = module_config,

src/modules/rlm_json/rlm_json.c

@@ -616,7 +616,6 @@ module_rlm_t rlm_json = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "json",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.onload		= mod_load,
 		.unload		= mod_unload,
 		.config		= module_config,

src/modules/rlm_krb5/rlm_krb5.c

@@ -493,9 +493,10 @@ module_rlm_t rlm_krb5 = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "krb5",
-#ifdef KRB5_IS_THREAD_SAFE
-		.flags		= MODULE_TYPE_THREAD_SAFE,
-#else
+		/*
+		 *	FIXME - Probably want a global mutex created on mod_load
+		 */
+#ifndef KRB5_IS_THREAD_SAFE
 		.flags		= MODULE_TYPE_THREAD_UNSAFE,
 #endif
 		.inst_size	= sizeof(rlm_krb5_t),

src/modules/rlm_lua/rlm_lua.c

@@ -169,7 +169,6 @@ module_rlm_t rlm_lua = {
 	.common = {
 		.magic			= MODULE_MAGIC_INIT,
 		.name			= "lua",
-		.flags			= MODULE_TYPE_THREAD_SAFE,
 		.inst_size		= sizeof(rlm_lua_t),
 
 		.thread_inst_size	= sizeof(rlm_lua_thread_t),

src/modules/rlm_opendirectory/rlm_opendirectory.c

@@ -534,7 +534,6 @@ module_rlm_t rlm_opendirectory = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "opendirectory",
 		.inst_size	= sizeof(rlm_opendirectory_t),
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.instantiate	= mod_instantiate
 	},
 	.method_names = (module_method_name_t[]){

src/modules/rlm_perl/rlm_perl.c

@@ -1170,7 +1170,6 @@ module_rlm_t rlm_perl = {
 	.common = {
 		.magic			= MODULE_MAGIC_INIT,
 		.name			= "perl",
-		.flags			= MODULE_TYPE_THREAD_SAFE,
 		.inst_size		= sizeof(rlm_perl_t),
 
 		.config			= module_config,

src/modules/rlm_python/rlm_python.c

@@ -1170,7 +1170,6 @@ module_rlm_t rlm_python = {
 	.common = {
 		.magic			= MODULE_MAGIC_INIT,
 		.name			= "python",
-		.flags			= MODULE_TYPE_THREAD_SAFE,
 
 		.inst_size		= sizeof(rlm_python_t),
 		.thread_inst_size	= sizeof(rlm_python_thread_t),

src/modules/rlm_radius/rlm_radius.c

@@ -655,7 +655,6 @@ module_rlm_t rlm_radius = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "radius",
-		.flags		= MODULE_TYPE_THREAD_SAFE | MODULE_TYPE_RESUMABLE,
 		.inst_size	= sizeof(rlm_radius_t),
 		.config		= module_config,
 

src/modules/rlm_redis/rlm_redis.c

@@ -900,7 +900,6 @@ module_rlm_t rlm_redis = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "redis",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_redis_t),
 		.config		= module_config,
 		.onload		= mod_load,

src/modules/rlm_redis_ippool/rlm_redis_ippool.c

@@ -1286,7 +1286,6 @@ module_rlm_t rlm_redis_ippool = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "redis",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_redis_ippool_t),
 		.config		= module_config,
 		.onload		= mod_load,

src/modules/rlm_rediswho/rlm_rediswho.c

@@ -252,7 +252,6 @@ module_rlm_t rlm_rediswho = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "rediswho",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_rediswho_t),
 		.config		= module_config,
 		.onload		= mod_load,

src/modules/rlm_rest/rlm_rest.c

@@ -1393,7 +1393,6 @@ module_rlm_t rlm_rest = {
 	.common = {
 		.magic			= MODULE_MAGIC_INIT,
 		.name			= "rest",
-		.flags			= MODULE_TYPE_THREAD_SAFE,
 		.inst_size		= sizeof(rlm_rest_t),
 		.thread_inst_size	= sizeof(rlm_rest_thread_t),
 		.config			= module_config,

src/modules/rlm_sigtran/rlm_sigtran.c

@@ -426,7 +426,6 @@ module_rlm_t rlm_sigtran = {
 	.common = {
 		.magic			= MODULE_MAGIC_INIT,
 		.name			= "sigtran",
-		.flags			= MODULE_TYPE_THREAD_SAFE | MODULE_TYPE_RESUMABLE,
 		.inst_size		= sizeof(rlm_sigtran_t),
 		.thread_inst_size	= sizeof(rlm_sigtran_thread_t),
 		.config			= module_config,

src/modules/rlm_smtp/rlm_smtp.c

@@ -1043,7 +1043,6 @@ module_rlm_t rlm_smtp = {
 	.common = {
 		.magic		        = MODULE_MAGIC_INIT,
 		.name		        = "smtp",
-		.flags		        = MODULE_TYPE_THREAD_SAFE,
 		.inst_size	        = sizeof(rlm_smtp_t),
 		.thread_inst_size   	= sizeof(rlm_smtp_thread_t),
 		.config		        = module_config,

src/modules/rlm_sql/rlm_sql.c

@@ -1937,7 +1937,6 @@ module_rlm_t rlm_sql = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "sql",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_sql_t),
 		.config		= module_config,
 		.bootstrap	= mod_bootstrap,

src/modules/rlm_sqlcounter/rlm_sqlcounter.c

@@ -579,7 +579,6 @@ module_rlm_t rlm_sqlcounter = {
 	.common = {
 		.magic		= MODULE_MAGIC_INIT,
 		.name		= "sqlcounter",
-		.flags		= MODULE_TYPE_THREAD_SAFE,
 		.inst_size	= sizeof(rlm_sqlcounter_t),
 		.config		= module_config,
 		.bootstrap	= mod_bootstrap,