diff --git a/cron/certificate_refresher.php b/cron/certificate_refresher.php index 4bdb04de..f8df97c5 100644 --- a/cron/certificate_refresher.php +++ b/cron/certificate_refresher.php @@ -8,6 +8,10 @@ if (php_sapi_name() !== 'cli') { die("This script must be run from the command line.\n"); } +// Prevent overlapping runs of this script +$cron_lock_script = __FILE__; +require_once "../includes/cron_lock.php"; + require_once "../config.php"; // Set Timezone diff --git a/cron/cron.php b/cron/cron.php index 604d4dba..98b609f3 100644 --- a/cron/cron.php +++ b/cron/cron.php @@ -8,18 +8,9 @@ if (php_sapi_name() !== 'cli') { die("This script must be run from the command line.\n"); } -// Only one run at a time. Autopay charges cards, so an overlapping run (the previous -// run still going when the next one fires) could bill the same invoice twice. The lock -// file is named per install so separate ITFlow instances on one host don't block each -// other, and the handle is held for the life of the process, released when it exits. -$cron_lock_file = sys_get_temp_dir() . '/itflow_cron_' . md5(__DIR__) . '.lock'; -$cron_lock_handle = fopen($cron_lock_file, 'c'); -if ($cron_lock_handle === false) { - die("Cannot open the cron lock file at $cron_lock_file - check permissions and open_basedir.\n"); -} -if (!flock($cron_lock_handle, LOCK_EX | LOCK_NB)) { - die("Cron is already running - exiting.\n"); -} +// Prevent overlapping runs of this script +$cron_lock_script = __FILE__; +require_once "../includes/cron_lock.php"; require_once "../config.php"; diff --git a/cron/domain_refresher.php b/cron/domain_refresher.php index 0412b1e5..abaa7c0e 100644 --- a/cron/domain_refresher.php +++ b/cron/domain_refresher.php @@ -8,6 +8,10 @@ if (php_sapi_name() !== 'cli') { die("This script must be run from the command line.\n"); } +// Prevent overlapping runs of this script +$cron_lock_script = __FILE__; +require_once "../includes/cron_lock.php"; + require_once "../config.php"; // Set Timezone diff --git a/cron/mail_queue.php b/cron/mail_queue.php index 180889c2..a3e443d5 100644 --- a/cron/mail_queue.php +++ b/cron/mail_queue.php @@ -7,6 +7,10 @@ if (php_sapi_name() !== 'cli') { die("This script must be run from the command line.\n"); } +// Prevent overlapping runs of this script +$cron_lock_script = __FILE__; +require_once "../includes/cron_lock.php"; + require_once "../config.php"; require_once "../includes/inc_set_timezone.php"; require_once "../functions.php"; @@ -81,25 +85,6 @@ if (empty($config_smtp_provider)) { exit(0); } -/** ======================================================================= - * Lock file - * ======================================================================= */ -$temp_dir = sys_get_temp_dir(); -$lock_file_path = "{$temp_dir}/itflow_mail_queue_{$installation_id}.lock"; - -if (file_exists($lock_file_path)) { - $file_age = time() - filemtime($lock_file_path); - if ($file_age > 600) { - unlink($lock_file_path); - logApp("Cron-Mail-Queue", "warning", "Cron Mail Queue detected a lock file was present but was over 10 minutes old so it removed it."); - } else { - logApp("Cron-Mail-Queue", "info", "Cron Mail Queue attempted to execute but was already executing so instead it terminated."); - exit("Script is already running. Exiting."); - } -} - -file_put_contents($lock_file_path, "Locked"); - /** ======================================================================= * Mail OAuth helpers + sender function * ======================================================================= */ @@ -248,6 +233,9 @@ function sendQueueEmail( $mail->isSMTP(); $mail->Host = $host; $mail->Port = $port; + // Bound the SMTP conversation. Without this an unresponsive mail server can + // hold the cron lock open indefinitely and stall the whole queue. + $mail->Timeout = 30; $enc = strtolower($encryption); if ($enc === '' || $enc === 'none') { @@ -301,6 +289,26 @@ function sendQueueEmail( return true; } +/** ======================================================================= + * RECOVER: status = 1 (Sending) left behind by a run that died + * + * Nothing else in the codebase ever selects status 1, so without this a row + * claimed by a run that was killed mid-send stays 'Sending' forever and is never + * delivered. The cron lock above guarantees no other run of this script is in + * progress, so any row still sitting at status 1 is by definition orphaned and + * safe to reclaim. It is moved to failed rather than queued so it inherits the + * retry pass's 30 minute backoff and attempt cap instead of retrying instantly. + * + * This can re-send a message that did go out but died before being marked sent. + * That trade is deliberate: a duplicate is recoverable, an invoice that silently + * never arrives is not. + * ======================================================================= */ +mysqli_query($mysqli, "UPDATE email_queue SET email_status = 2, email_failed_at = NOW(), email_attempts = email_attempts + 1 WHERE email_status = 1"); +$orphaned_emails = mysqli_affected_rows($mysqli); +if ($orphaned_emails > 0) { + logApp("Cron-Mail-Queue", "warning", "Recovered $orphaned_emails email(s) left in a sending state by a previous run - queued for retry."); +} + /** ======================================================================= * SEND: status = 0 (Queued) * ======================================================================= */ @@ -464,8 +472,3 @@ if (mysqli_num_rows($sql_failed_queue) > 0) { } } } - -/** ======================================================================= - * Unlock - * ======================================================================= */ -unlink($lock_file_path); diff --git a/cron/ticket_email_parser.php b/cron/ticket_email_parser.php index 17a2ae1a..119a5bf5 100644 --- a/cron/ticket_email_parser.php +++ b/cron/ticket_email_parser.php @@ -15,6 +15,10 @@ if (php_sapi_name() !== 'cli') { die("This script must be run from the command line.\n"); } +// Prevent overlapping runs of this script +$cron_lock_script = __FILE__; +require_once "../includes/cron_lock.php"; + // Autoload (Webklex & any composer deps) require_once "../libs/vendor/autoload.php"; diff --git a/includes/cron_lock.php b/includes/cron_lock.php new file mode 100644 index 00000000..e77c1f3e --- /dev/null +++ b/includes/cron_lock.php @@ -0,0 +1,38 @@ +