From 6bc6c49090aee8e4c506975d56a1171ab87320d7 Mon Sep 17 00:00:00 2001 From: selul Date: Tue, 21 Jul 2026 13:19:46 +0300 Subject: [PATCH 1/6] chore: remove PR announcer workflow [skip ci] Co-Authored-By: Claude Fable 5 --- .github/workflows/pr-announcer-docs.yml | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 .github/workflows/pr-announcer-docs.yml diff --git a/.github/workflows/pr-announcer-docs.yml b/.github/workflows/pr-announcer-docs.yml deleted file mode 100644 index 0892ecf50..000000000 --- a/.github/workflows/pr-announcer-docs.yml +++ /dev/null @@ -1,18 +0,0 @@ -on: - pull_request: - types: [closed] - branches: - - 'development' - - 'new/**' -jobs: - pr_announcer: - runs-on: ubuntu-latest - name: Announce pr - steps: - - name: Checking merged commit - uses: Codeinwp/action-pr-merged-announcer@main - env: - GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }} - with: - destination_repo: "Codeinwp/docs" - issue_labels: "optimole" From 6d4b46fb6ee39d09d681750ffdd7c67384f503ae Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Tue, 21 Jul 2026 17:15:25 +0530 Subject: [PATCH 2/6] fix: prevent duplicate scheduling of image processing actions --- inc/media_offload.php | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/inc/media_offload.php b/inc/media_offload.php index 999555313..de30699b7 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -1899,18 +1899,21 @@ public static function move_images( $action, $refresh ) { 'action' => $type, ]; } - $total = ceil( $count / $batch ); - self::schedule_action( - time(), - 'optml_start_processing_images', - [ - $action, - $batch, - 1, - $total, - $step, - ] - ); + + if ( ! self::is_scheduled( 'optml_start_processing_images' ) ) { + $total = ceil( $count / $batch ); + self::schedule_action( + time(), + 'optml_start_processing_images', + [ + $action, + $batch, + 1, + $total, + $step, + ] + ); + } } $response = [ From c7b0a476e665834ff7b1296e09d1fdf49b05d60d Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Tue, 21 Jul 2026 18:52:24 +0530 Subject: [PATCH 3/6] fix: avoid duplicate image processing jobs --- inc/media_offload.php | 195 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 191 insertions(+), 4 deletions(-) diff --git a/inc/media_offload.php b/inc/media_offload.php index de30699b7..0c4adbbf8 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -56,6 +56,17 @@ class Optml_Media_Offload extends Optml_App_Replacer { const POST_OFFLOADED_FLAG = 'optimole_offload_post'; const POST_ROLLBACK_FLAG = 'optimole_rollback_post'; const RETRYABLE_META_COUNTER = '_optimole_retryable_errors'; + + /** + * Option name for the transfer lock. + */ + const TRANSFER_LOCK_OPTION = 'optml_transfer_lock'; + + /** + * Time to live for the transfer lock, in seconds. + */ + const TRANSFER_LOCK_TTL = 600; + /** * Flag used inside wp_get_attachment url filter. * @@ -173,7 +184,7 @@ public static function instance() { add_filter( 'wp_insert_attachment_data', [ self::$instance, 'insert' ], 10, 4 ); } - add_action( 'optml_start_processing_images', [ self::$instance, 'start_processing_images' ], 10, 5 ); + add_action( 'optml_start_processing_images', [ self::$instance, 'start_processing_images' ], 10, 6 ); add_action( 'optml_move_images_by_id', [ @@ -201,7 +212,7 @@ public static function instance() { */ public function maybe_reschedule() { // If this is in pending, we do nothing. - if ( self::is_scheduled( 'optml_start_processing_images' ) ) { + if ( self::is_transfer_lock_active() ) { return; } // If there is no transfer in progress, we do nothing. @@ -1900,7 +1911,10 @@ public static function move_images( $action, $refresh ) { ]; } - if ( ! self::is_scheduled( 'optml_start_processing_images' ) ) { + // We acquire a lock to prevent multiple workers from running the same action concurrently. + $lock_token = self::acquire_transfer_lock( $action ); + + if ( false !== $lock_token ) { $total = ceil( $count / $batch ); self::schedule_action( time(), @@ -1911,6 +1925,7 @@ public static function move_images( $action, $refresh ) { 1, $total, $step, + $lock_token, ] ); } @@ -1973,6 +1988,165 @@ public static function is_scheduled( $hook ) { } } + /** + * Get the current transfer lock, if any. + * + * @return array{token: string, action: string, expires: int}|null Null if no lock is currently held. + */ + private static function get_transfer_lock() { + $value = get_option( self::TRANSFER_LOCK_OPTION ); + + if ( ! is_array( $value ) || ! isset( $value['token'], $value['expires'] ) ) { + return null; + } + + return $value; + } + + /** + * Check if the transfer lock is currently active (held by some worker and not expired). + * + * @return bool + */ + public static function is_transfer_lock_active() { + $lock = self::get_transfer_lock(); + + return null !== $lock && $lock['expires'] >= time(); + } + + /** + * Attempt to acquire the transfer lock for a given action. + * + * @param string $action The transfer action ('offload_images'|'rollback_images'). + * + * @return string|false The lock token on success, false if another worker holds it. + */ + public static function acquire_transfer_lock( $action ) { + global $wpdb; + + for ( $attempt = 0; $attempt < 2; $attempt++ ) { + $token = wp_generate_uuid4(); + $value = [ + 'token' => $token, + 'action' => $action, + 'expires' => time() + self::TRANSFER_LOCK_TTL, + ]; + + $suppress = $wpdb->suppress_errors( true ); + + // Attempt to insert a new lock row. If another worker has already inserted one, this will fail. + $inserted = $wpdb->insert( + $wpdb->options, + [ + 'option_name' => self::TRANSFER_LOCK_OPTION, + 'option_value' => maybe_serialize( $value ), + 'autoload' => 'no', + ] + ); + $wpdb->suppress_errors( $suppress ); + + if ( $inserted ) { + wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); + return $token; + } + + $existing = self::get_transfer_lock(); + + if ( null === $existing ) { + continue; + } + + if ( $existing['expires'] >= time() ) { + return false; + } + + $updated = $wpdb->update( + $wpdb->options, + [ 'option_value' => maybe_serialize( $value ) ], + [ + 'option_name' => self::TRANSFER_LOCK_OPTION, + 'option_value' => maybe_serialize( $existing ), + ] + ); + + if ( $updated ) { + wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); + return $token; + } + } + + return false; + } + + /** + * Renew the transfer lock if we still own it, extending its expiration. + * + * @param string $token The lock token this worker was given when it started the chain. + * @param string $action The transfer action currently being processed. + * + * @return bool True if we still own the lock and renewed it, false if ownership was lost. + */ + public static function renew_transfer_lock( $token, $action ) { + global $wpdb; + + $existing = self::get_transfer_lock(); + + if ( null === $existing || $existing['token'] !== $token ) { + return false; + } + + $value = $existing; + $value['action'] = $action; + $value['expires'] = time() + self::TRANSFER_LOCK_TTL; + + $updated = $wpdb->update( + $wpdb->options, + [ 'option_value' => maybe_serialize( $value ) ], + [ + 'option_name' => self::TRANSFER_LOCK_OPTION, + 'option_value' => maybe_serialize( $existing ), + ] + ); + + if ( $updated ) { + wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); + return true; + } + + return false; + } + + /** + * Release the transfer lock if we still own it, allowing another worker to acquire it. + * + * @param string $token The lock token to release. + * + * @return void + */ + public static function release_transfer_lock( $token ) { + global $wpdb; + + if ( empty( $token ) ) { + return; + } + + $existing = self::get_transfer_lock(); + + if ( null === $existing || $existing['token'] !== $token ) { + return; + } + + $wpdb->delete( + $wpdb->options, + [ + 'option_name' => self::TRANSFER_LOCK_OPTION, + 'option_value' => maybe_serialize( $existing ), + ] + ); + + wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); + } + /** * Start Processing Images by IDs * @@ -2032,14 +2206,21 @@ public function move_single_image( $action, $id ) { * @param int $page The page of images to process. * @param int $total The total number of pages. * @param int $step The current step. + * @param string $lock_token The transfer lock token owned by this processing chain. * * @return void */ - public function start_processing_images( $action, $batch, $page, $total, $step ) { + public function start_processing_images( $action, $batch, $page, $total, $step, $lock_token = '' ) { $option = 'offload_images' === $action ? 'offloading_status' : 'rollback_status'; $type = 'offload_images' === $action ? 'offload' : 'rollback'; if ( self::$instance->settings->get( $option ) === 'disabled' ) { + self::release_transfer_lock( $lock_token ); + return; + } + + // If we don't own the lock anymore, stop processing. + if ( ! self::renew_transfer_lock( $lock_token, $action ) ) { return; } @@ -2051,6 +2232,9 @@ public function start_processing_images( $action, $batch, $page, $total, $step ) self::$instance->settings->update( 'show_offload_finish_notice', $type ); + // Transfer completed successfully: release the lock. + self::release_transfer_lock( $lock_token ); + return; } @@ -2083,10 +2267,12 @@ public function start_processing_images( $action, $batch, $page, $total, $step ) $page, $total, $step, + $lock_token, ] ); } catch ( Exception $e ) { // Reschedule the cron to run again after a delay. Sometimes memory limit is exausted. + // This is a retryable error, so the lock is kept rather than released. $delay_in_seconds = 10; self::$instance->logger->add_log( $type, $e->getMessage() ); @@ -2099,6 +2285,7 @@ public function start_processing_images( $action, $batch, $page, $total, $step ) $page, $total, $step, + $lock_token, ] ); } From 6e9577ee47e32f55848b252ea23df1e9aa5b7ac9 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Wed, 22 Jul 2026 15:09:15 +0530 Subject: [PATCH 4/6] fix: enhance transfer lock handling to prevent concurrent processing --- inc/media_offload.php | 80 ++++++++++++++----------------- tests/test-media.php | 106 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 45 deletions(-) diff --git a/inc/media_offload.php b/inc/media_offload.php index 0c4adbbf8..099d3a2af 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -2024,58 +2024,49 @@ public static function is_transfer_lock_active() { public static function acquire_transfer_lock( $action ) { global $wpdb; - for ( $attempt = 0; $attempt < 2; $attempt++ ) { - $token = wp_generate_uuid4(); - $value = [ - 'token' => $token, - 'action' => $action, - 'expires' => time() + self::TRANSFER_LOCK_TTL, - ]; - - $suppress = $wpdb->suppress_errors( true ); + $existing = self::get_transfer_lock(); - // Attempt to insert a new lock row. If another worker has already inserted one, this will fail. - $inserted = $wpdb->insert( + if ( null !== $existing && $existing['expires'] < time() ) { + $wpdb->delete( $wpdb->options, [ 'option_name' => self::TRANSFER_LOCK_OPTION, - 'option_value' => maybe_serialize( $value ), - 'autoload' => 'no', + 'option_value' => maybe_serialize( $existing ), ] ); - $wpdb->suppress_errors( $suppress ); - - if ( $inserted ) { - wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); - return $token; - } + wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); + } - $existing = self::get_transfer_lock(); + $token = wp_generate_uuid4(); + $value = [ + 'token' => $token, + 'action' => $action, + 'expires' => time() + self::TRANSFER_LOCK_TTL, + ]; - if ( null === $existing ) { - continue; - } + $inserted = $wpdb->query( // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $wpdb->prepare( + "INSERT IGNORE INTO `$wpdb->options` (`option_name`, `option_value`, `autoload`) VALUES (%s, %s, 'no')", + self::TRANSFER_LOCK_OPTION, + maybe_serialize( $value ) + ) + ); - if ( $existing['expires'] >= time() ) { - return false; - } + if ( ! $inserted ) { + return false; + } - $updated = $wpdb->update( - $wpdb->options, - [ 'option_value' => maybe_serialize( $value ) ], - [ - 'option_name' => self::TRANSFER_LOCK_OPTION, - 'option_value' => maybe_serialize( $existing ), - ] - ); + wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); - if ( $updated ) { - wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); - return $token; - } + // Clear the `notoptions` cache so that get_option() will see + // the new lock immediately instead of waiting for the next cache refresh. + $notoptions = wp_cache_get( 'notoptions', 'options' ); + if ( is_array( $notoptions ) && isset( $notoptions[ self::TRANSFER_LOCK_OPTION ] ) ) { + unset( $notoptions[ self::TRANSFER_LOCK_OPTION ] ); + wp_cache_set( 'notoptions', $notoptions, 'options' ); } - return false; + return $token; } /** @@ -2099,7 +2090,7 @@ public static function renew_transfer_lock( $token, $action ) { $value['action'] = $action; $value['expires'] = time() + self::TRANSFER_LOCK_TTL; - $updated = $wpdb->update( + $wpdb->update( $wpdb->options, [ 'option_value' => maybe_serialize( $value ) ], [ @@ -2108,12 +2099,11 @@ public static function renew_transfer_lock( $token, $action ) { ] ); - if ( $updated ) { - wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); - return true; - } + wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); + + $current = self::get_transfer_lock(); - return false; + return null !== $current && $current['token'] === $token; } /** diff --git a/tests/test-media.php b/tests/test-media.php index 4552dd912..e70d60fd5 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -639,4 +639,110 @@ public function test_alter_attachment_image_src() { } } } + + /** + * Two overlapping "start transfer" requests must not be able to claim the transfer lock. + */ + public function test_acquire_transfer_lock_blocks_concurrent_start() { + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + + $first = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); + $second = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); + + $this->assertNotFalse( $first ); + $this->assertFalse( $second ); + + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + } + + /** + * A lock left behind by a worker that died mid-batch (fatal error, killed process) must + * eventually become reclaimable, otherwise a crashed transfer would be stuck forever. + */ + public function test_acquire_transfer_lock_reclaims_expired_lock() { + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + + $first = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); + $this->assertNotFalse( $first ); + + // Simulate a worker that died mid-batch: force the lock's expiry into the past. + $stale = get_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + $stale['expires'] = time() - 10; + update_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION, $stale, false ); + + $this->assertFalse( Optml_Media_Offload::is_transfer_lock_active() ); + + $second = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); + + $this->assertNotFalse( $second ); + $this->assertNotEquals( $first, $second ); + + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + } + + /** + * Only the current lock owner (matching token) may renew it - otherwise a worker that + * already lost the lock to a reclaiming worker could revive its stale ownership. + */ + public function test_renew_transfer_lock_requires_owned_token() { + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + + $token = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); + $this->assertNotFalse( $token ); + + $this->assertFalse( Optml_Media_Offload::renew_transfer_lock( 'not-the-owner', 'offload_images' ) ); + + $lock_before = get_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + + $this->assertTrue( Optml_Media_Offload::renew_transfer_lock( $token, 'offload_images' ) ); + + $lock_after = get_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + // >= rather than a strict >: a renewal computed within the same wall-clock second as the + // original acquire can legitimately land on the same expiry timestamp. + $this->assertGreaterThanOrEqual( $lock_before['expires'], $lock_after['expires'] ); + + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + } + + /** + * Only the current lock owner (matching token) may release it - a stray/late release call + * from a worker that already lost ownership must not tear down another worker's lock. + */ + public function test_release_transfer_lock_only_releases_owned_lock() { + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + + $token = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); + $this->assertNotFalse( $token ); + + Optml_Media_Offload::release_transfer_lock( 'not-the-owner' ); + $this->assertTrue( Optml_Media_Offload::is_transfer_lock_active() ); + + Optml_Media_Offload::release_transfer_lock( $token ); + $this->assertFalse( Optml_Media_Offload::is_transfer_lock_active() ); + } + + /** + * Test that two overlapping move_images() calls do not schedule two processing chains for the same transfer. + */ + public function test_move_images_does_not_schedule_duplicate_processing_chain() { + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + + Optml_Media_Offload::instance()->settings->update( 'rollback_status', 'enabled' ); + Optml_Media_Offload::instance()->settings->update( 'offloading_status', 'disabled' ); + + Optml_Media_Offload::move_images( 'rollback_images', false ); + Optml_Media_Offload::move_images( 'rollback_images', false ); + + // Count the number of scheduled processing chains for the transfer. There should be exactly one. + $scheduled_count = 0; + foreach ( _get_cron_array() as $timestamp => $hooks ) { + if ( isset( $hooks['optml_start_processing_images'] ) ) { + $scheduled_count += count( $hooks['optml_start_processing_images'] ); + } + } + + $this->assertEquals( 1, $scheduled_count ); + + delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + } } \ No newline at end of file From f1ad1417e9421a260c6c4f222a27712b7c6f40ea Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Wed, 22 Jul 2026 17:32:01 +0530 Subject: [PATCH 5/6] fix: prevent duplicate scheduling --- inc/media_offload.php | 128 +++++++++--------------------------------- tests/test-media.php | 32 +++++------ 2 files changed, 38 insertions(+), 122 deletions(-) diff --git a/inc/media_offload.php b/inc/media_offload.php index 099d3a2af..57cae606c 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -58,9 +58,9 @@ class Optml_Media_Offload extends Optml_App_Replacer { const RETRYABLE_META_COUNTER = '_optimole_retryable_errors'; /** - * Option name for the transfer lock. + * Transient name for the transfer lock. */ - const TRANSFER_LOCK_OPTION = 'optml_transfer_lock'; + const TRANSFER_LOCK_TRANSIENT = 'optml_transfer_lock'; /** * Time to live for the transfer lock, in seconds. @@ -1988,84 +1988,30 @@ public static function is_scheduled( $hook ) { } } - /** - * Get the current transfer lock, if any. - * - * @return array{token: string, action: string, expires: int}|null Null if no lock is currently held. - */ - private static function get_transfer_lock() { - $value = get_option( self::TRANSFER_LOCK_OPTION ); - - if ( ! is_array( $value ) || ! isset( $value['token'], $value['expires'] ) ) { - return null; - } - - return $value; - } - - /** - * Check if the transfer lock is currently active (held by some worker and not expired). - * - * @return bool - */ - public static function is_transfer_lock_active() { - $lock = self::get_transfer_lock(); - - return null !== $lock && $lock['expires'] >= time(); - } - /** * Attempt to acquire the transfer lock for a given action. * * @param string $action The transfer action ('offload_images'|'rollback_images'). * - * @return string|false The lock token on success, false if another worker holds it. + * @return string|false The lock token on success, false if another worker already holds the lock. */ public static function acquire_transfer_lock( $action ) { - global $wpdb; - - $existing = self::get_transfer_lock(); - - if ( null !== $existing && $existing['expires'] < time() ) { - $wpdb->delete( - $wpdb->options, - [ - 'option_name' => self::TRANSFER_LOCK_OPTION, - 'option_value' => maybe_serialize( $existing ), - ] - ); - wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); + $lock = get_transient( self::TRANSFER_LOCK_TRANSIENT ); + if ( false !== $lock ) { + return false; } $token = wp_generate_uuid4(); - $value = [ - 'token' => $token, - 'action' => $action, - 'expires' => time() + self::TRANSFER_LOCK_TTL, - ]; - $inserted = $wpdb->query( // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared - $wpdb->prepare( - "INSERT IGNORE INTO `$wpdb->options` (`option_name`, `option_value`, `autoload`) VALUES (%s, %s, 'no')", - self::TRANSFER_LOCK_OPTION, - maybe_serialize( $value ) - ) + set_transient( + self::TRANSFER_LOCK_TRANSIENT, + [ + 'token' => $token, + 'action' => $action, + ], + self::TRANSFER_LOCK_TTL ); - if ( ! $inserted ) { - return false; - } - - wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); - - // Clear the `notoptions` cache so that get_option() will see - // the new lock immediately instead of waiting for the next cache refresh. - $notoptions = wp_cache_get( 'notoptions', 'options' ); - if ( is_array( $notoptions ) && isset( $notoptions[ self::TRANSFER_LOCK_OPTION ] ) ) { - unset( $notoptions[ self::TRANSFER_LOCK_OPTION ] ); - wp_cache_set( 'notoptions', $notoptions, 'options' ); - } - return $token; } @@ -2078,32 +2024,22 @@ public static function acquire_transfer_lock( $action ) { * @return bool True if we still own the lock and renewed it, false if ownership was lost. */ public static function renew_transfer_lock( $token, $action ) { - global $wpdb; - - $existing = self::get_transfer_lock(); + $lock = get_transient( self::TRANSFER_LOCK_TRANSIENT ); - if ( null === $existing || $existing['token'] !== $token ) { + if ( ! is_array( $lock ) || ! isset( $lock['token'] ) || $lock['token'] !== $token ) { return false; } - $value = $existing; - $value['action'] = $action; - $value['expires'] = time() + self::TRANSFER_LOCK_TTL; - - $wpdb->update( - $wpdb->options, - [ 'option_value' => maybe_serialize( $value ) ], + set_transient( + self::TRANSFER_LOCK_TRANSIENT, [ - 'option_name' => self::TRANSFER_LOCK_OPTION, - 'option_value' => maybe_serialize( $existing ), - ] + 'token' => $token, + 'action' => $action, + ], + self::TRANSFER_LOCK_TTL ); - wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); - - $current = self::get_transfer_lock(); - - return null !== $current && $current['token'] === $token; + return true; } /** @@ -2114,27 +2050,13 @@ public static function renew_transfer_lock( $token, $action ) { * @return void */ public static function release_transfer_lock( $token ) { - global $wpdb; - - if ( empty( $token ) ) { - return; - } - - $existing = self::get_transfer_lock(); + $lock = get_transient( self::TRANSFER_LOCK_TRANSIENT ); - if ( null === $existing || $existing['token'] !== $token ) { + if ( ! is_array( $lock ) || ! isset( $lock['token'] ) || $lock['token'] !== $token ) { return; } - $wpdb->delete( - $wpdb->options, - [ - 'option_name' => self::TRANSFER_LOCK_OPTION, - 'option_value' => maybe_serialize( $existing ), - ] - ); - - wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' ); + delete_transient( self::TRANSFER_LOCK_TRANSIENT ); } /** diff --git a/tests/test-media.php b/tests/test-media.php index e70d60fd5..128c5414c 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -644,7 +644,7 @@ public function test_alter_attachment_image_src() { * Two overlapping "start transfer" requests must not be able to claim the transfer lock. */ public function test_acquire_transfer_lock_blocks_concurrent_start() { - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); $first = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); $second = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); @@ -652,7 +652,7 @@ public function test_acquire_transfer_lock_blocks_concurrent_start() { $this->assertNotFalse( $first ); $this->assertFalse( $second ); - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); } /** @@ -660,15 +660,13 @@ public function test_acquire_transfer_lock_blocks_concurrent_start() { * eventually become reclaimable, otherwise a crashed transfer would be stuck forever. */ public function test_acquire_transfer_lock_reclaims_expired_lock() { - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); $first = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); $this->assertNotFalse( $first ); - // Simulate a worker that died mid-batch: force the lock's expiry into the past. - $stale = get_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); - $stale['expires'] = time() - 10; - update_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION, $stale, false ); + // Simulate a worker that died mid-batch: force the transient's timeout into the past. + update_option( '_transient_timeout_' . Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT, time() - 10 ); $this->assertFalse( Optml_Media_Offload::is_transfer_lock_active() ); @@ -677,7 +675,7 @@ public function test_acquire_transfer_lock_reclaims_expired_lock() { $this->assertNotFalse( $second ); $this->assertNotEquals( $first, $second ); - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); } /** @@ -685,23 +683,19 @@ public function test_acquire_transfer_lock_reclaims_expired_lock() { * already lost the lock to a reclaiming worker could revive its stale ownership. */ public function test_renew_transfer_lock_requires_owned_token() { - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); $token = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); $this->assertNotFalse( $token ); $this->assertFalse( Optml_Media_Offload::renew_transfer_lock( 'not-the-owner', 'offload_images' ) ); - $lock_before = get_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); - $this->assertTrue( Optml_Media_Offload::renew_transfer_lock( $token, 'offload_images' ) ); - $lock_after = get_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); - // >= rather than a strict >: a renewal computed within the same wall-clock second as the - // original acquire can legitimately land on the same expiry timestamp. - $this->assertGreaterThanOrEqual( $lock_before['expires'], $lock_after['expires'] ); + $lock_after = get_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + $this->assertSame( $token, $lock_after['token'] ); - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); } /** @@ -709,7 +703,7 @@ public function test_renew_transfer_lock_requires_owned_token() { * from a worker that already lost ownership must not tear down another worker's lock. */ public function test_release_transfer_lock_only_releases_owned_lock() { - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); $token = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); $this->assertNotFalse( $token ); @@ -725,7 +719,7 @@ public function test_release_transfer_lock_only_releases_owned_lock() { * Test that two overlapping move_images() calls do not schedule two processing chains for the same transfer. */ public function test_move_images_does_not_schedule_duplicate_processing_chain() { - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); Optml_Media_Offload::instance()->settings->update( 'rollback_status', 'enabled' ); Optml_Media_Offload::instance()->settings->update( 'offloading_status', 'disabled' ); @@ -743,6 +737,6 @@ public function test_move_images_does_not_schedule_duplicate_processing_chain() $this->assertEquals( 1, $scheduled_count ); - delete_option( Optml_Media_Offload::TRANSFER_LOCK_OPTION ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); } } \ No newline at end of file From 85a4db1961388770cd2d1c4b9564d050a513b783 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Wed, 22 Jul 2026 17:47:37 +0530 Subject: [PATCH 6/6] fix: update transfer lock check --- inc/media_offload.php | 3 ++- tests/test-media.php | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/inc/media_offload.php b/inc/media_offload.php index 57cae606c..cd66df44f 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -211,8 +211,9 @@ public static function instance() { * @return void */ public function maybe_reschedule() { + $lock = get_transient( self::TRANSFER_LOCK_TRANSIENT ); // If this is in pending, we do nothing. - if ( self::is_transfer_lock_active() ) { + if ( false !== $lock ) { return; } // If there is no transfer in progress, we do nothing. diff --git a/tests/test-media.php b/tests/test-media.php index 128c5414c..529b5373b 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -668,7 +668,7 @@ public function test_acquire_transfer_lock_reclaims_expired_lock() { // Simulate a worker that died mid-batch: force the transient's timeout into the past. update_option( '_transient_timeout_' . Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT, time() - 10 ); - $this->assertFalse( Optml_Media_Offload::is_transfer_lock_active() ); + $this->assertFalse( false !== get_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ) ); $second = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); @@ -709,10 +709,10 @@ public function test_release_transfer_lock_only_releases_owned_lock() { $this->assertNotFalse( $token ); Optml_Media_Offload::release_transfer_lock( 'not-the-owner' ); - $this->assertTrue( Optml_Media_Offload::is_transfer_lock_active() ); + $this->assertTrue( false !== get_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ) ); Optml_Media_Offload::release_transfer_lock( $token ); - $this->assertFalse( Optml_Media_Offload::is_transfer_lock_active() ); + $this->assertFalse( false !== get_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ) ); } /**