Skip to content

RTC: Fix race condition on room creation which can cause a split update log #77675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dmsnell merged 11 commits into from
May 6, 2026
Merged

RTC: Fix race condition on room creation which can cause a split update log #77675

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a1ed827
Add sync storage race regression tests
danluu Apr 25, 2026
a02b370
Fix sync storage first writer race
danluu Apr 25, 2026
71d8a6b
Add backport reference
dmsnell Apr 27, 2026
288b052
Add cursor backfill race repros
danluu May 5, 2026
468fd7e
Update test docs, make some test helpers static.
dmsnell May 5, 2026
6d662b6
Make much more code very better
dmsnell May 5, 2026
d7a81ef
Fix cursor-safe duplicate sync storage repair
danluu May 5, 2026
8cc08a6
Fix cursor backfill repro assertions
danluu May 5, 2026
620f2df
Reduce RTC room split fix to safe scope
danluu May 6, 2026
a871f1c
Use `get_posts()` instead of a direct query
dmsnell May 6, 2026
3a531c6
Leave TODO note on wp_update_post()
dmsnell May 6, 2026
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
3 changes: 3 additions & 0 deletions backport-changelog/7.0/11660.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
https://github.com/WordPress/wordpress-develop/pull/11660

* https://github.com/WordPress/gutenberg/pull/77675
207 changes: 199 additions & 8 deletions lib/compat/wordpress-7.0/class-wp-sync-post-meta-storage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,16 @@ class WP_Sync_Post_Meta_Storage implements WP_Sync_Storage {
public function add_update( string $room, $update ): bool {
global $wpdb;

$post_id = $this->get_storage_post_id( $room );
$room_hash = md5( $room );
$post_id = $this->get_storage_post_id( $room );
if ( null === $post_id ) {
return false;
}

// Use direct database operation to avoid cache invalidation performed by
// post meta functions (`wp_cache_set_posts_last_changed()` and direct
// `wp_cache_delete()` calls).
return (bool) $wpdb->insert(
$result = $wpdb->insert(
$wpdb->postmeta,
array(
'post_id' => $post_id,
Expand All @@ -97,6 +98,12 @@ public function add_update( string $room, $update ): bool {
),
array( '%d', '%s', '%s' )
);

if ( $result ) {
self::$storage_post_ids[ $room_hash ] = $this->merge_duplicate_storage_posts( $room_hash, $post_id );
}

return (bool) $result;
}

/**
Expand Down Expand Up @@ -155,7 +162,8 @@ public function get_awareness_state( string $room ): array {
public function set_awareness_state( string $room, array $awareness ): bool {
global $wpdb;

$post_id = $this->get_storage_post_id( $room );
$room_hash = md5( $room );
$post_id = $this->get_storage_post_id( $room );
if ( null === $post_id ) {
return false;
}
Expand All @@ -176,16 +184,22 @@ public function set_awareness_state( string $room, array $awareness ): bool {
);

if ( $meta_id ) {
return (bool) $wpdb->update(
$result = $wpdb->update(
$wpdb->postmeta,
array( 'meta_value' => wp_json_encode( $awareness ) ),
array( 'meta_id' => $meta_id ),
array( '%s' ),
array( '%d' )
);

if ( false !== $result ) {
self::$storage_post_ids[ $room_hash ] = $this->merge_duplicate_storage_posts( $room_hash, $post_id );
}

return false !== $result;
}

return (bool) $wpdb->insert(
$result = $wpdb->insert(
$wpdb->postmeta,
array(
'post_id' => $post_id,
Expand All @@ -194,6 +208,12 @@ public function set_awareness_state( string $room, array $awareness ): bool {
),
array( '%d', '%s', '%s' )
);

if ( $result ) {
self::$storage_post_ids[ $room_hash ] = $this->merge_duplicate_storage_posts( $room_hash, $post_id );
}

return (bool) $result;
}

/**
Expand Down Expand Up @@ -264,14 +284,185 @@ private function get_storage_post_id( string $room ): ?int {
)
);

if ( is_int( $post_id ) ) {
self::$storage_post_ids[ $room_hash ] = $post_id;
return $post_id;
if ( is_int( $post_id ) && $post_id > 0 ) {
$canonical_post_id = $this->resolve_canonical_storage_post_id_after_insert( $room_hash, $post_id );
if ( null === $canonical_post_id ) {
return null;
}

self::$storage_post_ids[ $room_hash ] = $canonical_post_id;
return $canonical_post_id;
}

return null;
}

/**
* Resolves the canonical room storage post after inserting a new post.
*
* Two concurrent first writers can both miss the lookup above and create
* storage posts for the same room hash. Depending on the exact interleaving,
* WordPress may create either a duplicate exact slug or a suffixed slug.
* When that happens, merge everything back into one canonical lineage.
*
* @since 7.0.0
*
* @param string $room_hash MD5 hash of the room identifier.
* @param int $inserted_post_id Post ID returned by wp_insert_post().
* @return int|null Canonical storage post ID.
*/
private function resolve_canonical_storage_post_id_after_insert( string $room_hash, int $inserted_post_id ): ?int {
$canonical_post_id = $this->find_canonical_storage_post_id( $room_hash );
if ( null === $canonical_post_id ) {
$canonical_post_id = $this->promote_storage_post_to_canonical_slug( $room_hash, $inserted_post_id );
}

if ( null === $canonical_post_id ) {
wp_delete_post( $inserted_post_id, true );
return null;
}

return $this->merge_duplicate_storage_posts( $room_hash, $canonical_post_id );
}

/**
* Merges duplicate storage posts created by a first-access race.
*
* @since 7.0.0
*
* @param string $room_hash MD5 hash of the room identifier.
* @param int $canonical_post_id Preferred post ID that should own the room.
* @return int Canonical storage post ID.
*/
private function merge_duplicate_storage_posts( string $room_hash, int $canonical_post_id ): int {
global $wpdb;

$storage_post_ids = $this->get_storage_post_ids_for_room_hash( $room_hash );
if ( empty( $storage_post_ids ) ) {
return $canonical_post_id;
}

$exact_post_id = $this->find_canonical_storage_post_id( $room_hash );
if ( null === $exact_post_id ) {
$canonical_post_id = in_array( $canonical_post_id, $storage_post_ids, true ) ? $canonical_post_id : (int) $storage_post_ids[0];
$promoted_post_id = $this->promote_storage_post_to_canonical_slug( $room_hash, $canonical_post_id );
if ( null === $promoted_post_id ) {
return $canonical_post_id;
}

$canonical_post_id = $promoted_post_id;
$storage_post_ids = $this->get_storage_post_ids_for_room_hash( $room_hash );
} else {
$canonical_post_id = $exact_post_id;
}

foreach ( $storage_post_ids as $duplicate_id ) {
if ( $canonical_post_id === $duplicate_id ) {
continue;
}

$move_result = $wpdb->update(
$wpdb->postmeta,
array( 'post_id' => $canonical_post_id ),
array( 'post_id' => $duplicate_id ),
array( '%d' ),
array( '%d' )
);

@peterwilsoncc peterwilsoncc Apr 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may result in two copies of awareness post_meta stored against a single post ID.

@dmsnell dmsnell May 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have the mechanism for how this would create the duplicates? we did some investigation into meta_id, but trying to prevent that ended up adding considerable complexity and was addressing a secondary effect of this PR, which itself is preventing duplicate posts at a more frequent rate.

I think that even in the presence of new risk here, my assessment is that this is significantly lowering the overall risk. that is, we’re not introducing the possibility of creating duplicate awareness posts; we’re starting in a system that already duplicates them and we’re removing one common way it does that.


if ( false === $move_result ) {
continue;
}

wp_delete_post( $duplicate_id, true );
}

return $canonical_post_id;
}

/**
* Finds the canonical storage post for a room hash.
*
* The canonical post is the oldest published storage post with the exact
* room hash slug. Suffixed slugs are repair candidates, not canonical.
*
* @since 7.0.0
*
* @param string $room_hash MD5 hash of the room identifier.
* @return int|null Canonical storage post ID.
*/
private function find_canonical_storage_post_id( string $room_hash ): ?int {
global $wpdb;

$post_id = $wpdb->get_var(
$wpdb->prepare(
"SELECT ID FROM {$wpdb->posts} WHERE post_type = %s AND post_status = 'publish' AND post_name = %s ORDER BY ID ASC LIMIT 1",
self::POST_TYPE,
$room_hash
)
);

@peterwilsoncc peterwilsoncc Apr 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can use the get_posts API.

$posts = get_posts(
				array(
					'post_type'      => self::POST_TYPE,
					'posts_per_page' => 1,
					'post_status'    => 'publish',
					'name'           => $room_hash,
					'fields'         => 'ids',
					'orderby'        => 'ID',
					'order'          => 'ASC',
				)
			);

@dmsnell dmsnell May 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @peterwilsoncc — I’ve made that update in a871f1c, but I wonder if there’s important gains running through all the additional machinery in get_posts(). given that this is going to be occurring frequently during an edit session.

I’ll leave it and we can see later if it’s problematic. if this code disappears in favor of a table then I suppose this is good enough.

@dmsnell dmsnell May 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops I changed the code to get_posts() but then I didn’t change the response handler, introducing a bug. @t-hamano caught it and resolved it in #78053


return is_numeric( $post_id ) ? (int) $post_id : null;
}

/**
* Promotes a storage post to the canonical room slug.
*
* @since 7.0.0
*
* @param string $room_hash MD5 hash of the room identifier.
* @param int $post_id Post ID to promote.
* @return int|null Promoted post ID on success.
*/
private function promote_storage_post_to_canonical_slug( string $room_hash, int $post_id ): ?int {
global $wpdb;

$result = $wpdb->update(
$wpdb->posts,
array( 'post_name' => $room_hash ),
array(
'ID' => $post_id,
'post_type' => self::POST_TYPE,
'post_status' => 'publish',
),
array( '%s' ),
array( '%d', '%s', '%s' )
);

if ( false === $result ) {
return null;
}

clean_post_cache( $post_id );

@dmsnell dmsnell Apr 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven’t followed the post-cache issues too closely, but I wonder if this is going to be problematic as well.

I would expect that promoting a non-canonical storage meta will occur rarely (vs. multiple times per second as was occurring in trunk for edits), but I would like some double-checking on this to make sure it would not cause thrashing on the site with other posts.

@peterwilsoncc this PR might seem a bit random, but are you able to speak to the use of clean_post_cache() here? the situation is that two sessions opened a post and due to a race condition, created two separate wp_sync_storage rows. this code is resolving that race.

@peterwilsoncc peterwilsoncc Apr 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can use wp_update_post()

@dmsnell dmsnell May 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @peterwilsoncc — I’ve made that update in a871f1c, but I wonder if there’s important gains running through all the additional machinery in get_posts(). given that this is going to be occurring frequently during an edit session.

I’ll leave it and we can see later if it’s problematic. if this code disappears in favor of a table then I suppose this is good enough.

return $post_id;
}

/**
* Lists storage posts belonging to a room hash, including suffixed duplicates.
*
* @since 7.0.0
*
* @param string $room_hash MD5 hash of the room identifier.
* @return array<int> Storage post IDs.
*/
private function get_storage_post_ids_for_room_hash( string $room_hash ): array {
global $wpdb;

$post_ids = $wpdb->get_col(
$wpdb->prepare(
"SELECT ID FROM {$wpdb->posts}
WHERE post_type = %s
AND post_status = 'publish'
AND ( post_name = %s OR post_name LIKE %s )
ORDER BY ID ASC",
self::POST_TYPE,
$room_hash,
$wpdb->esc_like( $room_hash . '-' ) . '%'
)

@dmsnell dmsnell Apr 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the concern may be minor here, but I wanted to ask if this OR clause is hiding a needless table scan. I asked an LLM to analyze it and it suggested that a UNION instead could introduce two index-scans to replace a single table scan.

Here is MariaDB’s EXPLAIN for the two versions:

MariaDB [wordpress]> EXPLAIN SELECT ID FROM wp_posts WHERE post_type = "wp_sync_storage" AND post_status = "publish" AND ( post_name = "abc123" OR post_name LIKE "abc123-" ) ORDER BY ID ASC;
+------+-------------+----------+-------+-----------------------------------------------+---------+---------+------+------+-------------+
| id   | select_type | table    | type  | possible_keys                                 | key     | key_len | ref  | rows | Extra       |
+------+-------------+----------+-------+-----------------------------------------------+---------+---------+------+------+-------------+
|    1 | SIMPLE      | wp_posts | index | post_name,type_status_date,type_status_author | PRIMARY | 8       | NULL | 35   | Using where |
+------+-------------+----------+-------+-----------------------------------------------+---------+---------+------+------+-------------+
1 row in set (0.003 sec)

MariaDB [wordpress]> EXPLAIN SELECT ID FROM wp_posts WHERE post_type = "wp_sync_storage" AND post_status = "publish" AND post_name = "abc123" UNION SELECT ID FROM wp_posts WHERE post_type = "wp_sync_storage" AND post_status = "publish" AND post_name LIKE "abc123-" ORDER BY ID ASC;
+------+--------------+------------+-------+-----------------------------------------------+-----------+---------+-------+------+----------------+
| id   | select_type  | table      | type  | possible_keys                                 | key       | key_len | ref   | rows | Extra          |
+------+--------------+------------+-------+-----------------------------------------------+-----------+---------+-------+------+----------------+
|    1 | PRIMARY      | wp_posts   | ref   | post_name,type_status_date,type_status_author | post_name | 766     | const | 1    | Using where    |
|    2 | UNION        | wp_posts   | range | post_name,type_status_date,type_status_author | post_name | 766     | NULL  | 1    | Using where    |
| NULL | UNION RESULT | <union1,2> | ALL   | NULL                                          | NULL      | NULL    | NULL  | NULL | Using filesort |
+------+--------------+------------+-------+-----------------------------------------------+-----------+---------+-------+------+----------------+
3 rows in set (0.002 sec)

Granted, for my little test site with two posts both are going to be roughly equivalent, but if we hit a site with thousands of posts, I wonder if this could introduce noticeable DB overhead.

Not being sure how to interpret the Using filesort I also asked about eliminating that by sorting in PHP and there was no clear guidance other than to say that when the expected set is small (which it is here) there’s no practical difference.

);

return array_map( 'intval', $post_ids );
}

/**
* Gets the number of updates stored for a given room.
*
Expand Down
Loading
Loading