Make WordPress Core

Opened 23 months ago

Last modified 19 months ago

#55952 new defect (bug)

add_option() racy behaviour

Reported by: karl94's profile karl94 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Options, Meta APIs Keywords: has-patch
Focuses: Cc:

Description

If two or more processes try to add an option via add_option() at the same time with both the same value, the following calls to get_option() may return false as if the option wasn't set.

This happens for example if two processes A and B: both reach past the check in https://github.com/WordPress/wordpress-develop/blob/6.0/src/wp-includes/option.php#L622-L624 before any of the two performs the INSERT.
In this scenario, the first process which gets to execute the INSERT will return true and will have the correct value in its object cache. The other process's add_option() will return false and it's object cache will continue to believe the option doesn't exists.

https://github.com/WordPress/wordpress-develop/blob/6.0/src/wp-includes/option.php#L640-L641
https://github.com/WordPress/wordpress-develop/blob/6.0/src/wp-includes/wp-db.php#L2116-L2131
https://dev.mysql.com/doc/refman/8.0/en/insert-on-duplicate.html#:~:text=0%20if%20an%20existing%20row%20is%20set%20to%20its%20current%20values

You can find attached a plugin designed for testing this race condition.

Attachments (3)

racy_add_option.php (2.1 KB) - added by karl94 23 months ago.
racy add_option.png (82.4 KB) - added by karl94 23 months ago.
55952-rfc.patch (3.6 KB) - added by karl94 23 months ago.

Download all attachments as: .zip

Change History (8)

#1 @SergeyBiryukov
23 months ago

Hi there, welcome back to WordPress Trac!

Just linking to some related tickets here: #38931, #51352, #51372.

Last edited 23 months ago by SergeyBiryukov (previous) (diff)

#2 @karl94
23 months ago

@SergeyBiryukov thanks, I think #51486 is also related.

I prepared a preliminary patch which takes into account both issues, could you please take a look at it and let me know what you think?

@karl94
23 months ago

#3 @SergeyBiryukov
23 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 6.1

Thanks for the patch! Moving to 6.1 for review.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


19 months ago

#5 @audrasjb
19 months ago

  • Milestone changed from 6.1 to Future Release

Unfortunately, this ticket didn’t get enough momentum in 6.1 release cycle… sorry about that, but the proposed patch still needs testing, so let's move it to Future Release milestone, at least until it's properly reviewed and tested.

Note: See TracTickets for help on using tickets.