Make WordPress Core

Opened 2 years ago

Last modified 2 years 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 2 years ago.
racy add_option.png (82.4 KB) - added by karl94 2 years ago.
55952-rfc.patch (3.6 KB) - added by karl94 2 years ago.

Download all attachments as: .zip

Change History (8)

#1 @SergeyBiryukov
2 years ago

Hi there, welcome back to WordPress Trac!

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

Last edited 2 years ago by SergeyBiryukov (previous) (diff)

#2 @karl94
2 years 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
2 years ago

#3 @SergeyBiryukov
2 years 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.


2 years ago

#5 @audrasjb
2 years 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.