fix: make the transfer lock atomic and clean up files left by interrupted rollbacks

inc/media_offload.php

@@ -57,6 +57,11 @@ class Optml_Media_Offload extends Optml_App_Replacer {
 	const POST_ROLLBACK_FLAG = 'optimole_rollback_post';
 	const RETRYABLE_META_COUNTER = '_optimole_retryable_errors';
 
+	/**
+	 * Post meta holding the file a rollback moved into uploads before the attachment was updated to it.
+	 */
+	const ROLLBACK_PENDING_FILE = '_optimole_rollback_pending_file';
+
 	/**
 	 * Transient name for the transfer lock.
 	 */
@@ -874,6 +879,8 @@ public function rollback_and_update_images( $image_ids ) {
 			if ( ! isset( $current_meta['file'] ) || ! self::is_uploaded_image( $current_meta['file'] ) ) {
 				delete_post_meta( $id, self::META_KEYS['offloaded'] );
 				delete_post_meta( $id, self::OM_OFFLOADED_FLAG );
+				// A rollback that died after WordPress saved the local metadata ends here on retry.
+				delete_post_meta( $id, self::ROLLBACK_PENDING_FILE );
 				++$success_back;
 				continue;
 			}
@@ -966,8 +973,21 @@ public function rollback_and_update_images( $image_ids ) {
 				continue;
 			}
 
-			// Move the temporary file into the uploads directory.
-			$results = wp_handle_sideload( $file, $overrides, get_the_date( 'Y/m', $id ) );
+			if ( ! function_exists( 'update_attached_file' ) ) {
+				include_once ABSPATH . '/wp-admin/includes/post.php';
+			}
+			if ( ! function_exists( 'update_attached_file' ) ) {
+				update_post_meta( $id, self::META_KEYS['rollback_error'], 'true' );
+				continue;
+			}
+
+			// A file left behind by an interrupted attempt is replaced in place: a new sideload would get a
+			// suffixed name, and deleting it first would leave the attachment without a file if the move fails.
+			$pending = self::get_pending_rollback_file( $id );
+			$results = null !== $pending
+				? self::replace_pending_rollback_file( $pending, $temp_file, $type )
+				// Move the temporary file into the uploads directory.
+				: wp_handle_sideload( $file, $overrides, get_the_date( 'Y/m', $id ) );
 			if ( ! empty( $results['error'] ) ) {
 				if ( OPTML_DEBUG_MEDIA ) {
 					do_action( 'optml_log', ' wp_handle_sideload error' );
@@ -978,6 +998,13 @@ public function rollback_and_update_images( $image_ids ) {
 				continue;
 			}
 
+			// Remember the moved file until the attachment points to it, so a retry can clean it up.
+			update_post_meta( $id, self::ROLLBACK_PENDING_FILE, $results['file'] );
+
+			// Point the attachment at the restored file right away: generating the sub-sizes is the
+			// slow part, and a request that dies there must not leave the stored path behind.
+			update_attached_file( $id, $results['file'] );
+
 			if ( ! function_exists( 'wp_create_image_subsizes' ) ) {
 				include_once ABSPATH . '/wp-admin/includes/image.php';
 			}
@@ -1003,14 +1030,7 @@ public function rollback_and_update_images( $image_ids ) {
 				wp_update_attachment_metadata( $id, $meta );
 			}
 
-			if ( ! function_exists( 'update_attached_file' ) ) {
-				include_once ABSPATH . '/wp-admin/includes/post.php';
-			}
-			if ( ! function_exists( 'update_attached_file' ) ) {
-				update_post_meta( $id, self::META_KEYS['rollback_error'], 'true' );
-				continue;
-			}
-			update_attached_file( $id, $results['file'] );
+			delete_post_meta( $id, self::ROLLBACK_PENDING_FILE );
 
 			$duplicated_images = apply_filters( 'optml_offload_duplicated_images', [], $id );
 			if ( is_array( $duplicated_images ) && ! empty( $duplicated_images ) ) {
@@ -1052,6 +1072,75 @@ public function rollback_and_update_images( $image_ids ) {
 		return $success_back;
 	}
 
+	/**
+	 * Get the file an interrupted rollback of this attachment moved into uploads.
+	 *
+	 * Only an existing file inside the uploads directory is returned, so a corrupted meta value can
+	 * never point the rollback at anything else.
+	 *
+	 * @param int $id The attachment ID.
+	 *
+	 * @return string|null The absolute path, or null when there is no usable file.
+	 */
+	public static function get_pending_rollback_file( int $id ): ?string {
+		$pending = get_post_meta( $id, self::ROLLBACK_PENDING_FILE, true );
+		if ( empty( $pending ) || ! is_string( $pending ) ) {
+			return null;
+		}
+
+		$uploads  = wp_get_upload_dir();
+		$base_dir = realpath( $uploads['basedir'] );
+		$file     = realpath( $pending );
+		if ( false === $base_dir || false === $file || ! is_file( $file ) ) {
+			return null;
+		}
+
+		$base_dir = trailingslashit( wp_normalize_path( $base_dir ) );
+		if ( strpos( wp_normalize_path( $file ), $base_dir ) !== 0 ) {
+			return null;
+		}
+
+		// The stored path, not the resolved one: WordPress stores attached files relative to the uploads path as configured.
+		return $pending;
+	}
+
+	/**
+	 * Put the downloaded image in place of the file an interrupted rollback left behind.
+	 *
+	 * The image is copied next to the old file first and then renamed over it, so a copy that fails
+	 * partway (full disk, file size limit) leaves the old file intact.
+	 *
+	 * @param string $pending The file left behind, from {@see self::get_pending_rollback_file()}.
+	 * @param string $temp_file The downloaded image.
+	 * @param string $type The image MIME type.
+	 *
+	 * @return array{file?: string, url?: string, type?: string, error?: string} The same shape as wp_handle_sideload().
+	 */
+	public static function replace_pending_rollback_file( string $pending, string $temp_file, string $type ): array {
+		$partial = $pending . '.optml-partial';
+		// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- a failed move is reported through the return value.
+		if ( ! @copy( $temp_file, $partial ) || ! @rename( $partial, $pending ) ) {
+			wp_delete_file( $partial );
+
+			return [ 'error' => 'The restored file could not be written to ' . $pending ];
+		}
+		wp_delete_file( $temp_file );
+
+		// Same permissions as wp_handle_sideload() gives a new file.
+		$stat = stat( dirname( $pending ) );
+		if ( false !== $stat ) {
+			chmod( $pending, $stat['mode'] & 0000666 );
+		}
+
+		$uploads = wp_get_upload_dir();
+
+		return [
+			'file' => $pending,
+			'url'  => str_replace( wp_normalize_path( $uploads['basedir'] ), $uploads['baseurl'], wp_normalize_path( $pending ) ),
+			'type' => $type,
+		];
+	}
+
 	/**
 	 * Handle the bulk actions.
 	 *
@@ -2069,25 +2158,79 @@ public static function is_scheduled( $hook ) {
 	 * @return string|false The lock token on success, false if another worker already holds the lock.
 	 */
 	public static function acquire_transfer_lock( $action ) {
+		// Reading the transient also removes it once it has expired, so an expired lock can be taken again.
 		$lock = get_transient( self::TRANSFER_LOCK_TRANSIENT );
 		if ( false !== $lock ) {
 			return false;
 		}
 
 		$token = wp_generate_uuid4();
+		$value = [
+			'token'  => $token,
+			'action' => $action,
+		];
 
-		set_transient(
-			self::TRANSFER_LOCK_TRANSIENT,
-			[
-				'token'  => $token,
-				'action' => $action,
-			],
-			self::TRANSFER_LOCK_TTL
-		);
+		if ( ! self::add_transfer_lock( $value ) ) {
+			return false;
+		}
 
 		return $token;
 	}
 
+	/**
+	 * Store the transfer lock only if no other request stored it first.
+	 *
+	 * A read followed by a write lets two requests that start at the same moment both take the lock.
+	 * The write here fails when the lock already exists: wp_cache_add() with a persistent object
+	 * cache, and an INSERT IGNORE on the transient row otherwise. Both use the same storage as
+	 * set_transient(), so the rest of the lock code reads it as before.
+	 *
+	 * @param array{token: string, action: string} $value The lock value.
+	 *
+	 * @return bool True if this request stored the lock.
+	 */
+	private static function add_transfer_lock( array $value ): bool {
+		if ( wp_using_ext_object_cache() ) {
+			return wp_cache_add( self::TRANSFER_LOCK_TRANSIENT, $value, 'transient', self::TRANSFER_LOCK_TTL );
+		}
+
+		global $wpdb;
+
+		// The expiry is written first, so a request that dies right after the insert cannot leave a
+		// lock that never expires. A request that loses the race only rewrites the winner's expiry to
+		// the same moment.
+		update_option( '_transient_timeout_' . self::TRANSFER_LOCK_TRANSIENT, time() + self::TRANSFER_LOCK_TTL, false );
+
+		$option = '_transient_' . self::TRANSFER_LOCK_TRANSIENT;
+		$added  = $wpdb->query(
+			$wpdb->prepare(
+				"INSERT IGNORE INTO `{$wpdb->options}` (`option_name`, `option_value`, `autoload`) VALUES (%s, %s, %s)",
+				$option,
+				maybe_serialize( $value ),
+				'no'
+			)
+		);
+
+		// The read above cached the transient as missing; forget that so the next read sees the new row.
+		$notoptions = wp_cache_get( 'notoptions', 'options' );
+		if ( is_array( $notoptions ) && isset( $notoptions[ $option ] ) ) {
+			unset( $notoptions[ $option ] );
+			wp_cache_set( 'notoptions', $notoptions, 'options' );
+		}
+		wp_cache_delete( $option, 'options' );
+
+		if ( 1 !== $added ) {
+			return false;
+		}
+
+		// Written again, in case the previous holder's release removed it between the two writes. The
+		// cached copy is dropped first: it still holds the first write, so update_option() would skip the write.
+		wp_cache_delete( '_transient_timeout_' . self::TRANSFER_LOCK_TRANSIENT, 'options' );
+		update_option( '_transient_timeout_' . self::TRANSFER_LOCK_TRANSIENT, time() + self::TRANSFER_LOCK_TTL, false );
+
+		return true;
+	}
+
 	/**
 	 * Renew the transfer lock if we still own it, extending its expiration.
 	 *