Prevent duplicate scheduling of image processing actions by girishpanchal30 · Pull Request #1094 · Codeinwp/optimole-wp

girishpanchal30 · 2026-07-21T11:48:33Z

All Submissions:

Have you followed the Optimole Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Added a check to verify whether the optml_start_processing_images action is already scheduled. If it isn't, the action is scheduled. This prevents multiple instances of the same action from being scheduled.

Closes #1074

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

Copilot

Pull request overview

Adds a scheduling guard intended to prevent duplicate image-processing jobs during media offload and rollback.

Changes:

Checks for an existing processing action before scheduling.
Leaves concurrent scheduling and running-job scenarios unresolved.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pirate-bot · 2026-07-21T11:51:15Z

Plugin build for 85a4db1 is ready 🛎️!

Download build
Download dev

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (6)

inc/media_offload.php:68

The lock can expire while a batch is still processing. A rollback batch defaults to 20 images, and each download_url() call can block for 60 seconds (inc/media_offload.php:905-906), so one batch can exceed this 600-second TTL. During that window maybe_reschedule() can replace the lock and launch another worker against the same images. Renew the lease during long batches (for example, per image) or use a locking design that cannot expire while the owner is actively processing.

	const TRANSFER_LOCK_TTL = 600;

inc/media_offload.php:2049

This direct insert does not clear WordPress's cached notoptions entry. maybe_reschedule() calls get_option() earlier on every request, so an absent lock is commonly negative-cached before this insert; with a persistent object cache, the scheduled worker will continue seeing no lock, fail renewal, and stop the transfer. Use add_option() for the atomic insert so WordPress maintains both the option and notoptions caches.

			if ( $inserted ) {
				wp_cache_delete( self::TRANSFER_LOCK_OPTION, 'options' );

inc/media_offload.php:1919

The initial scheduling result is ignored after the lock has been acquired. If Action Scheduler or WP-Cron rejects the event, no worker exists but the lock makes repeated starts and maybe_reschedule() no-op for up to ten minutes. Release the token when enqueueing fails so recovery can happen immediately.

				self::schedule_action(

inc/media_offload.php:1915

This concurrency fix has no tests for duplicate acquisition, ownership-safe renewal/release, expired-lock takeover, or a failed enqueue. These are the behaviors most likely to regress and are not covered by the existing media tests; add focused tests for the lock lifecycle and duplicate start path.

			// We acquire a lock to prevent multiple workers from running the same action concurrently.
			$lock_token = self::acquire_transfer_lock( $action );

inc/media_offload.php:2270

A failed continuation enqueue leaves the transfer lock active even though this worker is finished and no next worker exists. Since recovery now checks the lock rather than the scheduler, processing stalls until the lease expires. Check the return value and release the owned lock when scheduling fails.

					$lock_token,

inc/media_offload.php:2288

The retry enqueue can also fail while retaining the lock. In that case there is neither a pending retry nor immediate recovery because maybe_reschedule() sees an active lock. Release the token when scheduling the retry fails.

					$lock_token,

Soare-Robert-Daniel · 2026-07-21T13:34:39Z

+		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;
+	}


Is there a more elegant way than this?

Yes, if the existing lock has expired, we delete it as a best-effort operation. It doesn't matter if 0 rows are affected, as another worker may have already cleared or replaced the lock. We then always attempt a plain $wpdb->insert().

Since the outcome of the INSERT is the only thing that determines lock ownership, there is no need for a compare-and-swap (CAS)-conditioned UPDATE or a retry loop.

Please let me know if you have any other thoughts or suggestions.

Thanks!

I see that WordPress has a similar mechanism when upgrading https://developer.wordpress.org/reference/classes/wp_upgrader/create_lock/

Do you think we can create something similar based on their pattern (you can tell the AI to work based on it)? For the moment, the $wpdb->suppress_errors( $suppress ); does not inspire much safety.

And also, regarding the solution, we should add some tests to simulate this scenario.

I have implemented it as you suggested. Please check it and let me know if any changes are needed.

selul · 2026-07-22T09:42:33Z

@girishpanchal30 have you found out the root cause? the scheduling already has enough locking check via wp-cron/action scheduler, i see we reinvent a lot of stuff.

girishpanchal30 · 2026-07-22T11:21:58Z

@selul I wasn't able to reproduce the issue in my local environment. However, after reviewing the code, I identified the underlying cause: when number_of_images_and_pages() is called during a media offload, it schedules the optml_start_processing_images action without ensuring that another processing chain hasn't already been scheduled for the same transfer.

As suggested by Copilot, I implemented a locking mechanism that guards scheduling for both the offload_images and rollback_images actions. This ensures that only one processing chain can be scheduled per transfer, eliminating the race condition in which two concurrent requests could otherwise both schedule the same job before either completes.

selul · 2026-07-22T11:25:52Z

@girishpanchal30 in this case we can use a simple transient which clears up when the process finish and is setting up when the process start.

girishpanchal30 · 2026-07-22T12:03:37Z

@selul I have implemented transient as you suggested with the latest commit. Please check it and let me know if any changes are needed.

fix: prevent duplicate scheduling of image processing actions

6d4b46f

girishpanchal30 requested a review from Copilot July 21, 2026 11:48

Copilot started reviewing on behalf of girishpanchal30 July 21, 2026 11:49 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread inc/media_offload.php Outdated

Comment thread inc/media_offload.php Outdated

fix: avoid duplicate image processing jobs

c7b0a47

girishpanchal30 requested a review from Copilot July 21, 2026 13:22

Copilot started reviewing on behalf of girishpanchal30 July 21, 2026 13:23 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

girishpanchal30 requested a review from Soare-Robert-Daniel July 21, 2026 13:28

girishpanchal30 mentioned this pull request Jul 21, 2026

Media offload can enqueue duplicate optml_start_processing_images jobs #1074

Open

Soare-Robert-Daniel reviewed Jul 21, 2026

View reviewed changes

fix: enhance transfer lock handling to prevent concurrent processing

6e9577e

fix: prevent duplicate scheduling

f1ad141

fix: update transfer lock check

85a4db1

girishpanchal30 force-pushed the bugfix/1074 branch from 2bed02f to 85a4db1 Compare July 22, 2026 12:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Prevent duplicate scheduling of image processing actions#1094

Prevent duplicate scheduling of image processing actions#1094
girishpanchal30 wants to merge 5 commits into
developmentfrom
bugfix/1074

girishpanchal30 commented Jul 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

pirate-bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Soare-Robert-Daniel Jul 21, 2026

Uh oh!

girishpanchal30 Jul 21, 2026

Uh oh!

Soare-Robert-Daniel Jul 22, 2026

Uh oh!

girishpanchal30 Jul 22, 2026

Uh oh!

selul commented Jul 22, 2026

Uh oh!

girishpanchal30 commented Jul 22, 2026

Uh oh!

selul commented Jul 22, 2026

Uh oh!

girishpanchal30 commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

girishpanchal30 commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

Other information:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

pirate-bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Soare-Robert-Daniel Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

girishpanchal30 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Soare-Robert-Daniel Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

girishpanchal30 Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

selul commented Jul 22, 2026

Uh oh!

girishpanchal30 commented Jul 22, 2026

Uh oh!

selul commented Jul 22, 2026

Uh oh!

girishpanchal30 commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

girishpanchal30 commented Jul 21, 2026 •

edited

Loading

pirate-bot commented Jul 21, 2026 •

edited

Loading