Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions lib/media/class-gutenberg-rest-attachments-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,24 @@ public function sideload_item( WP_REST_Request $request ) {

$image_size = $request['image_size'];

// Acquire a per-attachment transient lock to prevent concurrent sideloads
// from overwriting each other's metadata changes (read-modify-write race).
// Uses the same transient lock pattern as wp_maybe_generate_attachment_metadata().
$lock_key = 'wp_sideload_att_' . $attachment_id;

if ( ! $this->acquire_metadata_lock( $lock_key ) ) {
// Clean up the uploaded file since we cannot update metadata.
wp_delete_file( $path );
return new WP_Error(
'rest_upload_lock_timeout',
__( 'Could not acquire metadata lock for this attachment. Please try again.', 'gutenberg' ),
array( 'status' => 503 )
);
}

// Invalidate the object cache to read the latest metadata from the database.
wp_cache_delete( $attachment_id, 'post_meta' );

$metadata = wp_get_attachment_metadata( $attachment_id, true );

if ( ! $metadata ) {
Expand Down Expand Up @@ -514,6 +532,8 @@ public function sideload_item( WP_REST_Request $request ) {

wp_update_attachment_metadata( $attachment_id, $metadata );

delete_transient( $lock_key );

$response_request = new WP_REST_Request(
WP_REST_Server::READABLE,
rest_get_route_for_post( $attachment_id )
Expand All @@ -531,4 +551,29 @@ public function sideload_item( WP_REST_Request $request ) {

return $response;
}

/**
* Acquires a transient-based lock for attachment metadata updates.
*
* Uses the same transient lock pattern as wp_maybe_generate_attachment_metadata()
* in WordPress core. Spins with brief pauses until the lock is available or
* the timeout is reached.
*
* @param string $lock_key Transient key for the lock.
* @param int $timeout Maximum wait time in seconds. Default 5.
* @return bool True if the lock was acquired, false on timeout.
*/
private function acquire_metadata_lock( string $lock_key, int $timeout = 5 ): bool {
$start = time();

while ( get_transient( $lock_key ) ) {
if ( ( time() - $start ) >= $timeout ) {
return false;
}
usleep( 50000 ); // 50ms.
}

set_transient( $lock_key, time(), $timeout );
return true;
}
Comment thread
adamsilverstein marked this conversation as resolved.
Outdated
}
4 changes: 4 additions & 0 deletions packages/upload-media/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Enhancement

- Remove sideload upload serialization: thumbnail uploads now run concurrently, governed by `maxConcurrentUploads` instead of being queued one-at-a-time per attachment ([#75257](https://github.com/WordPress/gutenberg/pull/75257)).

## 0.28.0 (2026-04-01)

## 0.27.0 (2026-03-18)
Expand Down
63 changes: 0 additions & 63 deletions packages/upload-media/src/store/private-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import type {
PauseQueueAction,
QueueItem,
QueueItemId,
ResumeItemAction,
ResumeQueueAction,
RevokeBlobUrlsAction,
SideloadAdditionalData,
Expand All @@ -62,8 +61,6 @@ type ActionCreators = {
addItem: typeof addItem;
addSideloadItem: typeof addSideloadItem;
removeItem: typeof removeItem;
pauseItem: typeof pauseItem;
resumeItemByPostId: typeof resumeItemByPostId;
prepareItem: typeof prepareItem;
processItem: typeof processItem;
finishOperation: typeof finishOperation;
Expand Down Expand Up @@ -94,32 +91,6 @@ type ThunkArgs = {
registry: WPDataRegistry;
};

/**
* Determines if an upload should be paused to avoid race conditions.
*
* When sideloading thumbnails, we need to pause uploads if another
* upload to the same post is already in progress.
*
* @param item Queue item to check.
* @param operation Current operation type.
* @param select Store selectors.
* @return Whether the upload should be paused.
*/
function shouldPauseForSideload(
item: QueueItem,
operation: OperationType | undefined,
select: Selectors
): boolean {
if (
operation !== OperationType.Upload ||
! item.parentId ||
! item.additionalData.post
) {
return false;
}
return select.isUploadingToPost( item.additionalData.post as number );
}

interface AddItemArgs {
// It should always be a File, but some consumers might still pass Blobs only.
file: File | Blob;
Expand Down Expand Up @@ -306,16 +277,6 @@ export function processItem( id: QueueItemId ) {
? item.operations[ 0 ][ 1 ]
: undefined;

// If we're sideloading a thumbnail, pause upload to avoid race conditions.
// It will be resumed after the previous upload finishes.
if ( shouldPauseForSideload( item, operation, select ) ) {
dispatch< PauseItemAction >( {
type: Type.PauseItem,
id,
} );
return;
}

/*
* If the next operation is an upload, check concurrency limit.
* If at capacity, the item remains queued and will be processed
Expand Down Expand Up @@ -516,28 +477,6 @@ export function pauseItem( id: QueueItemId ) {
};
}

/**
* Resumes processing for a given post/attachment ID.
*
* This function looks up paused uploads by post ID and resumes them.
* It's typically called after a sideload completes to resume paused
* thumbnail uploads.
*
* @param postOrAttachmentId Post or attachment ID.
*/
export function resumeItemByPostId( postOrAttachmentId: number ) {
return async ( { select, dispatch }: ThunkArgs ) => {
const item = select.getPausedUploadForPost( postOrAttachmentId );
if ( item ) {
dispatch< ResumeItemAction >( {
type: Type.ResumeItem,
id: item.id,
} );
dispatch.processItem( item.id );
}
};
}

/**
* Removes a specific item from the queue.
*
Expand Down Expand Up @@ -850,11 +789,9 @@ export function sideloadItem( id: QueueItemId ) {
signal: item.abortController?.signal,
onFileChange: ( [ attachment ] ) => {
dispatch.finishOperation( id, { attachment } );
dispatch.resumeItemByPostId( post as number );
},
onError: ( error ) => {
dispatch.cancelItem( id, error );
dispatch.resumeItemByPostId( post as number );
},
} );
};
Expand Down
39 changes: 0 additions & 39 deletions packages/upload-media/src/store/private-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import {
type BatchId,
ItemStatus,
OperationType,
type QueueItem,
type QueueItemId,
Expand Down Expand Up @@ -51,44 +50,6 @@ export function isBatchUploaded( state: State, batchId: BatchId ): boolean {
return batchItems.length === 0;
}

/**
* Determines whether an upload is currently in progress given a post or attachment ID.
*
* @param state Upload state.
* @param postOrAttachmentId Post ID or attachment ID.
*
* @return Whether upload is currently in progress for the given post or attachment.
*/
export function isUploadingToPost(
state: State,
postOrAttachmentId: number
): boolean {
return state.queue.some(
( item ) =>
item.currentOperation === OperationType.Upload &&
item.additionalData.post === postOrAttachmentId
);
}

/**
* Returns the next paused upload for a given post or attachment ID.
*
* @param state Upload state.
* @param postOrAttachmentId Post ID or attachment ID.
*
* @return Paused item.
*/
export function getPausedUploadForPost(
state: State,
postOrAttachmentId: number
): QueueItem | undefined {
return state.queue.find(
( item ) =>
item.status === ItemStatus.Paused &&
item.additionalData.post === postOrAttachmentId
);
}

/**
* Determines whether uploading is currently paused.
*
Expand Down
Loading
Loading