can not update option in wp 3.7 and hanging in wp_clear_scheduled_hook()

[dimagsv]

update_option() must update option in memory cache and in database, but updates only in database. New option value not updated in memory cache when value in database is equal to new option value bacause zero (0) rows affected and update_option() return error (false).
wp_clear_scheduled_hook() hanging because cannot delete value from 'cron' option due this bug in update_option().
I think this code in update_option()

if ( ! $result )
 return false;

should be replaced with

if ( $result === false )
 return false;

[SergeyBiryukov]

Introduced in [25664].

[nacin]

New option value not updated in memory cache when value in database is equal to new option value bacause zero (0) rows affected and update_option() return error (false).

I think we need a test case. If the database had zero rows affected, then nothing was updated, and there is nothing to update in memcached.

[dimagsv]

How to reproduce.

1. Install ​Google XML Sitemaps plugin.
2. Activate plugin.
3. Enable option 'Rebuild sitemap if you change the content of your blog'.
4. Open in admin area any published page or post, press 'Save'.
5. Wait at least 15 seconds. Better 20 seconds. Do not open any other pages of this wordpress installation during this 15-20 seconds.
6. Press 'Save' again. WP hangs at wp_clear_scheduled_hook().

What is happen.

1. The event 'sm_build_cron' was scheduled on current time + 15 seconds after the first press 'Save'.
2. Options was read from database with cron scheduled event 'sm_build_cron' after the second press 'Save'.
3. Cron job started in the new thread.
4. Cron job in the new thread clear event 'sm_build_cron' from database earlier than wp_clear_scheduled_hook() called in the main thread.
5. Event 'sm_build_cron' exist in the main thread memory cache, but not in the database.

[dd32]

4. Cron job in the new thread clear event 'sm_build_cron' from database earlier than wp_clear_scheduled_hook() called in the main thread.
5. Event 'sm_build_cron' exist in the main thread memory cache, but not in the database.

So we've got 2 bugs here, based around the same thing,

1. If wp_clear_scheduled_hook() has been run in another thread, wp_clear_scheduled_hook() will endlessly loop in the current thread, because:
2. update_option() may now fail in "high-traffic" situations where an option is being updated my multiple threads to the same value.

There's multiple fixes:

1. Revert [25664], but that has the disadvantage that a DB write failing will still end up in the cache
2. make wp_clear_scheduled_hook() not rely upon wp_next_scheduled(), resulting it only trying to unset/save once-per-event rather than looping (because the cache says it's still there after "clearing" it)
3. make wp_next_scheduled() bypass the cache when it calls _get_cron_array() so that it's always got the latest data from the DB, downside that it's not as performant
4. make wp_clear_scheduled_hook aware that wp_unschedule_event() failed (but that also means making wp_unschedule_event() aware that _set_cron_array() can fail, and making _set_cron_array() aware that update_option can fail)
5. In the event that update_option() fails, populate the local cache with fresh data from the DB - this is technically possible, but also potentially weird, in that update_option might fail due to a DB issue, and then read stale data from a slave DB, or another write between the attempted write and the read causes the value to change to another one.

It's a tricky situation, as WordPress generally relies upon the fact that data it's using won't be altered outside of the current thread while it's using it.. which is a really bad operating assumption to make.

A unit test might be hard for this scenario, but should be able to replicate it by modifying the DB directly..

[kurtpayne]

I cannot reproduce this using the instructions given. I assume, based on the description, that there is an object cache present.

Replying to dd32:

So we've got 2 bugs here, based around the same thing,

1. If wp_clear_scheduled_hook() has been run in another thread, wp_clear_scheduled_hook() will endlessly loop in the current thread, because:

Is there a reason that we can't just make crons uncacheable? Something like this (don't cringe)

add_filter( 'pre_option_crons', function() {
    wp_cache_delete( 'cron', 'options' );
    return false;
}, -PHP_INT_MAX );

2. update_option() may now fail in "high-traffic" situations where an option is being updated my multiple threads to the same value.

Definitely can replicate this. If there is a multiple cache situation (like fastcgi + APC) then caches will get out of sync with each other and database very easily.

A unit test might be hard for this scenario, but should be able to replicate it by modifying the DB directly..

Yep. Let's focus on the cron issue, then move on to the bigger caching issue in another ticket. If you can help me reproduce this, I can start writing some tests around it.

[dd32]

In 26782:

Cron: Fix a case where a cache inconsistency can cause wp_clear_scheduled_hook() to enter an infinite loop. This unravels the function from using other cron api functions to looping over the cron array directly. See #25773

[nacin]

Looks great to me. dimagsv, any feedback?

[dimagsv]

Replying to kurtpayne:

I cannot reproduce this using the instructions given.

I have tried to reproduce this bug on an other hosting. Unsuccessfully. I think timing is important in this case. Special test plugin is needed to make this bug reproducible.

Replying to nacin:

Looks great to me. dimagsv, any feedback?

This will avoid hanging. Main issue will be fixed.

[nacin]

Hi dimagsv, this ticket was fixed in 3.8, but we're leaving this ticket around in case we do a 3.7.2 release. Thanks!

[nacin]

In 27886:

Cron: Fix a case where a cache inconsistency can cause wp_clear_scheduled_hook() to enter an infinite loop.

Merges [26782] from 3.8 to the 3.7 branch.

props dd32.
fixes #25773.