Refactor the email shutdown function

Register email_shutdown_function() from a new generic helper API,
mantis_shutdown_functions_register(); this could be used to add other
shutdown functions or a plugin event in the future.

The shutdown function is now always registered, and the logic to
determine whether to call email_send_all() or not has been moved into
it. This allows removal of the delay implemented in the previous
version to avoid concurrent executions.

The $g_email_stored global variable has been renamed and is now used as
a binary flag which drives the shutdown function's behavior. By setting
this flag, the request can
- indicate that emails have been generated, allowing the shutdown
  function to trigger email_send_all() only when necessary
- force sending of emails in the shutdown function, regardless of
  $g_email_send_using_cronjob setting (useful e.g. for signup or
  password reset processes, where we don't want to delay notifications).

To implement that behavior, a new optional parameter ($p_force) was
added to the email_store() function.

These changes are derived from vboctor's comments in PR 589.

Issue #17460

core.php

@@ -220,10 +220,8 @@ function __autoload( $p_class ) {
 	}
 }
 
-# Register shutdown function when processing emails synchronously
-if( OFF == config_get( 'email_send_using_cronjob' ) ) {
-	email_shutdown_function_register();
-}
+# Register global shutdown function
+shutdown_functions_register();
 
 # Initialise plugins
 if( !defined( 'PLUGINS_DISABLED' ) && !defined( 'MANTIS_MAINTENANCE_MODE' ) ) {

core/constant_inc.php

@@ -563,6 +563,11 @@
 define( 'PHPMAILER_METHOD_SENDMAIL', 1 );
 define( 'PHPMAILER_METHOD_SMTP', 2 );
 
+# Binary flag values for $g_email_shutdown_processing
+define( 'EMAIL_SHUTDOWN_SKIP', 0 );
+define( 'EMAIL_SHUTDOWN_GENERATED', 1 );
+define( 'EMAIL_SHUTDOWN_FORCE', 2 );
+
 # Lengths - NOTE: these may represent hard-coded values in db schema and should not be changed.
 define( 'DB_FIELD_SIZE_USERNAME', 255 );
 define( 'DB_FIELD_SIZE_REALNAME', 255 );

core/email_api.php

@@ -79,9 +79,16 @@
 # reusable object of class SMTP
 $g_phpMailer = null;
 
-# Indicates whether any emails are currently stored for process during this request.
-# Note: This is only used if not sending emails via cron job
-$g_email_stored = false;
+/**
+ * Indicates how generated emails will be processed by the shutdown function
+ * at the end of the current request's execution; this is a binary flag:
+ * - EMAIL_SHUTDOWN_SKIP       Initial state: do nothing (no generated emails)
+ * - EMAIL_SHUTDOWN_GENERATED  Emails will be sent, unless $g_email_send_using_cronjob is ON
+ * - EMAIL_SHUTDOWN_FORCE      All queued emails will be sent regardless of cronjob settings
+ * @see email_shutdown_function()
+ * @global $g_email_shutdown_processing
+ */
+$g_email_shutdown_processing = EMAIL_SHUTDOWN_SKIP;
 
 /**
  * Use a simple perl regex for valid email addresses.  This is not a complete regex,
@@ -468,7 +475,7 @@ function email_signup( $p_user_id, $p_password, $p_confirm_hash, $p_admin_name =
 	# Send signup email regardless of mail notification pref
 	# or else users won't be able to sign up
 	if( !is_blank( $t_email ) ) {
-		email_store( $t_email, $t_subject, $t_message );
+		email_store( $t_email, $t_subject, $t_message, null, true );
 		log_event( LOG_EMAIL, 'Signup Email = %s, Hash = %s, User = @U%d', $t_email, $p_confirm_hash, $p_user_id );
 	}
 
@@ -500,7 +507,7 @@ function email_send_confirm_hash_url( $p_user_id, $p_confirm_hash ) {
 	# Send password reset regardless of mail notification preferences
 	# or else users won't be able to receive their reset passwords
 	if( !is_blank( $t_email ) ) {
-		email_store( $t_email, $t_subject, $t_message );
+		email_store( $t_email, $t_subject, $t_message, null, true );
 		log_event( LOG_EMAIL, 'Password reset for user @U%d sent to %s', $p_user_id, $t_email );
 	}
 
@@ -734,14 +741,16 @@ function email_relationship_child_resolved_closed( $p_bug_id, $p_message_id ) {
 /**
  * Store email in queue for sending
  *
- * @param string $p_recipient Email recipient address.
- * @param string $p_subject   Subject of email message.
- * @param string $p_message   Body text of email message.
- * @param array  $p_headers   Array of additional headers to send with the email.
+ * @param string  $p_recipient Email recipient address.
+ * @param string  $p_subject   Subject of email message.
+ * @param string  $p_message   Body text of email message.
+ * @param array   $p_headers   Array of additional headers to send with the email.
+ * @param boolean $p_force     True to force sending of emails in shutdown function,
+ *                             even when using cronjob
  * @return integer|null
  */
-function email_store( $p_recipient, $p_subject, $p_message, array $p_headers = null ) {
-	global $g_email_stored;
+function email_store( $p_recipient, $p_subject, $p_message, array $p_headers = null, $p_force = false ) {
+	global $g_email_shutdown_processing;
 
 	$t_recipient = trim( $p_recipient );
 	$t_subject = string_email( trim( $p_subject ) );
@@ -778,7 +787,12 @@ function email_store( $p_recipient, $p_subject, $p_message, array $p_headers = n
 
 	$t_email_id = email_queue_add( $t_email_data );
 
-	$g_email_stored = true;
+	# Set the email processing flag for the shutdown function
+	if( $p_force ) {
+		$g_email_shutdown_processing |= EMAIL_SHUTDOWN_FORCE;
+	} else {
+		$g_email_shutdown_processing |= EMAIL_SHUTDOWN_GENERATED;
+	}
 
 	return $t_email_id;
 }
@@ -1396,50 +1410,34 @@ function email_build_visible_bug_data( $p_user_id, $p_bug_id, $p_message_id ) {
 	return $t_bug_data;
 }
 
-/**
- * Registers shutdown function to send queued emails after script execution
- * @return void
- */
-function email_shutdown_function_register() {
-	$t_register = false;
-	if( php_sapi_name() == 'cli' ) {
-		# Skip processing for predefined scripts
-		switch( basename( $_SERVER['SCRIPT_NAME'] ) ) {
-			case 'send_emails.php':
-				break;
-			default:
-				$t_register = true;
-		}
-	} else {
-		# Define shutdown function registration time with an arbitrary delay
-		# to prevent concurrent calls when multiple, parallel http requests
-		# are sent by a single user action
-		$t_now = time();
-		$t_registration_time = session_get_int( 'email_shutdown_function_set', $t_now) ;
-
-		if( $t_registration_time <= $t_now  ) {
-			$t_register = true;
-			# 5 seconds delay until the next registration
-			session_set( 'email_shutdown_function_set', $t_now + 5 );
-		}
-	}
-
-	if( $t_register ) {
-		log_event( LOG_EMAIL, 'Registering shutdown function for ' . $_SERVER['SCRIPT_NAME'] );
-		register_shutdown_function( 'email_shutdown_function' );
-	}
-}
-
 /**
  * The email sending shutdown function
+ * Will send any queued emails, except when $g_email_send_using_cronjob = ON.
+ * If $g_email_shutdown_processing EMAIL_SHUTDOWN_FORCE flag is set, emails
+ * will be sent regardless of cronjob setting.
  * @return void
  */
 function email_shutdown_function() {
-	global $g_email_stored;
+	global $g_email_shutdown_processing;
+
+	# Nothing to do if
+	# - no emails have been generated in the current request
+	# - system is configured to use cron job (unless processing is forced)
+	if(    $g_email_shutdown_processing == EMAIL_SHUTDOWN_SKIP
+		|| (   !( $g_email_shutdown_processing & EMAIL_SHUTDOWN_FORCE )
+			&& config_get( 'email_send_using_cronjob' )
+		   )
+	) {
+		return;
+	}
 
-	log_event( LOG_EMAIL, 'Shutdown function called' );
+	$t_msg ='Shutdown function called for ' . $_SERVER['SCRIPT_NAME'];
+	if( $g_email_shutdown_processing & EMAIL_SHUTDOWN_FORCE ) {
+		$t_msg .= ' (email processing forced)';
+	}
+	log_event( LOG_EMAIL, $t_msg );
 
-	if( $g_email_stored ) {
+	if( $g_email_shutdown_processing ) {
 		email_send_all();
 	}
 }

core/helper_api.php

@@ -683,3 +683,12 @@ function helper_duration_to_minutes( $p_hhmm ) {
 
 	return (int)$t_min;
 }
+
+/**
+ * Global shutdown functions registration
+ * Registers shutdown functions
+ * @return void
+ */
+function shutdown_functions_register() {
+	register_shutdown_function( 'email_shutdown_function' );
+}

core/logging_api.php

@@ -169,9 +169,9 @@ function log_event( $p_level, $p_msg ) {
  */
 function log_print_to_page() {
 	if( config_get_global( 'log_destination' ) === 'page' && auth_is_user_authenticated() && access_has_global_level( config_get( 'show_log_threshold' ) ) ) {
-		global $g_log_events, $g_log_levels, $g_email_stored;
+		global $g_log_events, $g_log_levels, $g_email_shutdown_processing;
 
-		if( $g_email_stored ) {
+		if( $g_email_shutdown_processing ) {
 			email_send_all();
 		}