Merge pull request #474 from mrrobot47/feat/backup-locking

feat(backup): add global lock to prevent concurrent backup OOM crashes

Author: mrrobot47

src/helper/Site_Backup_Restore.php

@@ -41,6 +41,9 @@ class Site_Backup_Restore {
 	private $dash_error_type = 'unknown';
 	private $dash_error_code = 0;
 
+	// Global backup lock handle for serializing backups
+	private $global_backup_lock_handle = null;
+
 	public function __construct() {
 		$this->fs = new Filesystem();
 	}
@@ -97,6 +100,12 @@ public function backup( $args, $assoc_args = [] ) {
 			register_shutdown_function( [ $this, 'dash_shutdown_handler' ] );
 		}
 
+		// Acquire global lock to serialize backups (prevents OOM from concurrent backups)
+		$this->acquire_global_backup_lock();
+
+		// Register shutdown handler to release lock on any exit (error, crash, etc.)
+		register_shutdown_function( [ $this, 'release_global_backup_lock' ] );
+
 		$this->pre_backup_check();
 		$backup_dir = EE_BACKUP_DIR . '/' . $this->site_data['site_url'];
 
@@ -146,6 +155,9 @@ public function backup( $args, $assoc_args = [] ) {
 			}
 		}
 
+		// Release global backup lock (also released by shutdown handler as safety net)
+		$this->release_global_backup_lock();
+
 		delem_log( 'site backup end' );
 	}
 
@@ -992,7 +1004,7 @@ private function pre_restore_check() {
 		$this->pre_backup_restore_checks();
 
 		$remote_path = $this->get_remote_path( false );
-		$command     = sprintf( 'rclone size --json %s', $remote_path );
+		$command     = sprintf( 'rclone size --json %s', escapeshellarg( $remote_path ) );
 		$output      = EE::launch( $command );
 
 		if ( $output->return_code ) {
@@ -1169,7 +1181,7 @@ private function list_remote_backups( $return = false ) {
 
 		$remote_path = $this->get_rclone_config_path(); // Get remote path without creating a new timestamped folder
 
-		$command = sprintf( 'rclone lsf --dirs-only %s', $remote_path ); // List only directories
+		$command = sprintf( 'rclone lsf --dirs-only %s', escapeshellarg( $remote_path ) ); // List only directories
 		$output  = EE::launch( $command );
 
 		if ( $output->return_code !== 0 && ! $return ) {
@@ -1248,7 +1260,7 @@ private function get_remote_path( $upload = true ) {
 	private function rclone_download( $path ) {
 		$cpu_cores     = intval( EE::launch( 'nproc' )->stdout );
 		$multi_threads = min( intval( $cpu_cores ) * 2, 32 );
-		$command       = sprintf( "rclone copy -P --multi-thread-streams %d %s %s", $multi_threads, $this->get_remote_path( false ), $path );
+		$command       = sprintf( "rclone copy -P --multi-thread-streams %d %s %s", $multi_threads, escapeshellarg( $this->get_remote_path( false ) ), escapeshellarg( $path ) );
 		$output        = EE::launch( $command );
 
 		if ( $output->return_code ) {
@@ -1277,7 +1289,7 @@ private function rclone_upload( $path ) {
 			$s3_flag = ' --s3-chunk-size=64M --s3-upload-concurrency ' . min( intval( $cpu_cores ) * 2, 32 );
 		}
 
-		$command = sprintf( "rclone copy -P %s --transfers %d --checkers %d --buffer-size %s %s %s", $s3_flag, $transfers, $transfers, $buffer_size, $path, $this->get_remote_path() );
+		$command = sprintf( "rclone copy -P %s --transfers %d --checkers %d --buffer-size %s %s %s", $s3_flag, $transfers, $transfers, $buffer_size, escapeshellarg( $path ), escapeshellarg( $this->get_remote_path() ) );
 		$output  = EE::launch( $command );
 
 		if ( $output->return_code ) {
@@ -1289,7 +1301,7 @@ private function rclone_upload( $path ) {
 			EE::error( 'Error uploading backup to remote storage.' );
 		} else {
 
-			$command     = sprintf( 'rclone lsf %s', $this->get_remote_path( false ) );
+			$command     = sprintf( 'rclone lsf %s', escapeshellarg( $this->get_remote_path( false ) ) );
 			$output      = EE::launch( $command );
 			$remote_path = $output->stdout;
 			EE::success( 'Backup uploaded to remote storage. Remote path: ' . $remote_path );
@@ -1603,4 +1615,87 @@ private function sanitize_count( $value ) {
 
 		return intval( $value );
 	}
+
+	/**
+	 * Acquire a global backup lock to ensure only one backup runs at a time.
+	 * Uses flock() for atomic, race-condition-free locking.
+	 *
+	 * This prevents multiple concurrent backups from exhausting system resources
+	 * (RAM, CPU, disk I/O, network bandwidth) when triggered simultaneously.
+	 *
+	 * Note: flock() may not work reliably on NFS or other network filesystems.
+	 * EE_BACKUP_DIR should be on a local filesystem for proper lock behavior.
+	 *
+	 * @return void
+	 */
+	private function acquire_global_backup_lock() {
+		$lock_file = EE_BACKUP_DIR . '/backup-global.lock';
+		$max_wait  = 86400; // 24 hours max wait
+		$waited    = 0;
+		$interval  = 60;    // Check every 60 seconds
+
+		// Ensure backup directory exists
+		if ( ! $this->fs->exists( EE_BACKUP_DIR ) ) {
+			$this->fs->mkdir( EE_BACKUP_DIR );
+		}
+
+		// Open file handle (creates if doesn't exist)
+		$this->global_backup_lock_handle = fopen( $lock_file, 'c+' );
+
+		if ( ! $this->global_backup_lock_handle ) {
+			$this->capture_error(
+				'Cannot create backup lock file',
+				self::ERROR_TYPE_FILESYSTEM,
+				5002
+			);
+			EE::error( 'Cannot create backup lock file.' );
+		}
+
+		// Try to acquire exclusive lock (non-blocking first to log status)
+		while ( ! flock( $this->global_backup_lock_handle, LOCK_EX | LOCK_NB ) ) {
+			if ( $waited >= $max_wait ) {
+				fclose( $this->global_backup_lock_handle );
+				$this->global_backup_lock_handle = null;
+				$this->capture_error(
+					'Timeout waiting for another backup to complete',
+					self::ERROR_TYPE_LOCK,
+					5003
+				);
+				EE::error( 'Timeout waiting for another backup. Try again later.' );
+			}
+
+			// Read who has the lock
+			rewind( $this->global_backup_lock_handle );
+			$lock_info = stream_get_contents( $this->global_backup_lock_handle );
+
+			EE::log( sprintf( 'Another backup in progress (%s). Waiting... (%d/%d sec)',
+				trim( $lock_info ) ?: 'unknown', $waited, $max_wait ) );
+
+			sleep( $interval );
+			$waited += $interval;
+		}
+
+		// Got the lock! Write our info
+		ftruncate( $this->global_backup_lock_handle, 0 );
+		rewind( $this->global_backup_lock_handle );
+		fwrite( $this->global_backup_lock_handle, $this->site_data['site_url'] . ' (PID: ' . getmypid() . ')' );
+		fflush( $this->global_backup_lock_handle );
+
+		EE::debug( 'Acquired global backup lock for: ' . $this->site_data['site_url'] );
+	}
+
+	/**
+	 * Release the global backup lock.
+	 * Safe to call multiple times (idempotent).
+	 *
+	 * @return void
+	 */
+	public function release_global_backup_lock() {
+		if ( $this->global_backup_lock_handle ) {
+			flock( $this->global_backup_lock_handle, LOCK_UN );
+			fclose( $this->global_backup_lock_handle );
+			$this->global_backup_lock_handle = null;
+			EE::debug( 'Released global backup lock' );
+		}
+	}
 }