delete_option() does not clear cache if option is missing in database.

[emrikol]

It's an old one, but this is the mirror of #25015.

If, for some reason, an object does not exist in the database, but does exist in persistent object cache, calling delete_option() will fail and the cached option will continue to exist:

wp> get_option( 'test-option' );
=> bool(false)
wp> update_option( 'test-option', 'example', false );
=> bool(true)
wp> get_option( 'test-option' );
=> string(7) "example"
wp> global $wpdb;
wp> $wpdb->get_results( 'SELECT * FROM wp_options WHERE option_name="test-option"' );
=> array(1) {
  [0]=>
  object(stdClass)#1977 (4) {
    ["option_id"]=>
    string(3) "121"
    ["option_name"]=>
    string(11) "test-option"
    ["option_value"]=>
    string(7) "example"
    ["autoload"]=>
    string(2) "no"
  }
}
wp> $wpdb->delete( $wpdb->options, array( 'option_name' => 'test-option' ) );
=> int(1)
wp> $wpdb->get_results( 'SELECT * FROM wp_options WHERE option_name="test-option"' );
=> array(0) {
}
wp> get_option( 'test-option' );
=> string(7) "example"
wp> delete_option( 'test-option' );
=> bool(false)
wp> get_option( 'test-option' );
=> string(7) "example"

I know what you're thinking, "Don't ever delete directly on the database." But recently when I ran into this, manual deletion wasn't the cause. More than likely it was some sort of race condition between distributed database or cache servers, or other arcane edge cases. The end result though was that the cache and database were out of sync and subsequent automated delete_option() calls were silently failing to clear the cache.

The easiest solution would be to move the cache deletion before the database check:

diff --git a/src/wp-includes/option.php b/src/wp-includes/option.php
index 8692db7199..2f4a000ca1 100644
--- a/src/wp-includes/option.php
+++ b/src/wp-includes/option.php
@@ -648,6 +648,18 @@ function delete_option( $option ) {

        wp_protect_special_option( $option );

+       if ( ! wp_installing() ) {
+               if ( 'yes' === $row->autoload ) {
+                       $alloptions = wp_load_alloptions( true );
+                       if ( is_array( $alloptions ) && isset( $alloptions[ $option ] ) ) {
+                               unset( $alloptions[ $option ] );
+                               wp_cache_set( 'alloptions', $alloptions, 'options' );
+                       }
+               } else {
+                       wp_cache_delete( $option, 'options' );
+               }
+       }
+
        // Get the ID, if no ID then return.
        $row = $wpdb->get_row( $wpdb->prepare( "SELECT autoload FROM $wpdb->options WHERE option_name = %s", $option ) );
        if ( is_null( $row ) ) {
@@ -665,18 +677,6 @@ function delete_option( $option ) {

        $result = $wpdb->delete( $wpdb->options, array( 'option_name' => $option ) );

-       if ( ! wp_installing() ) {
-               if ( 'yes' === $row->autoload ) {
-                       $alloptions = wp_load_alloptions( true );
-                       if ( is_array( $alloptions ) && isset( $alloptions[ $option ] ) ) {
-                               unset( $alloptions[ $option ] );
-                               wp_cache_set( 'alloptions', $alloptions, 'options' );
-                       }
-               } else {
-                       wp_cache_delete( $option, 'options' );
-               }
-       }
-
        if ( $result ) {

                /**

but that's also before the delete_option hook. Instead, the cache purging logic block could be duplicated right before returning false:

diff --git a/src/wp-includes/option.php b/src/wp-includes/option.php
index 8692db7199..ab0a3f79f2 100644
--- a/src/wp-includes/option.php
+++ b/src/wp-includes/option.php
@@ -651,6 +651,19 @@ function delete_option( $option ) {
        // Get the ID, if no ID then return.
        $row = $wpdb->get_row( $wpdb->prepare( "SELECT autoload FROM $wpdb->options WHERE option_name = %s", $option ) );
        if ( is_null( $row ) ) {
+
+               if ( ! wp_installing() ) {
+                       if ( 'yes' === $row->autoload ) {
+                               $alloptions = wp_load_alloptions( true );
+                               if ( is_array( $alloptions ) && isset( $alloptions[ $option ] ) ) {
+                                       unset( $alloptions[ $option ] );
+                                       wp_cache_set( 'alloptions', $alloptions, 'options' );
+                               }
+                       } else {
+                               wp_cache_delete( $option, 'options' );
+                       }
+               }
+
                return false;
        }

but that's not very clean, with that much duplicate code.

I'm also open to suggestions if this is even a "core" bug, or if it should be the responsibility of the theme/plugin code to make sure the option cache is properly cleared?

[mdbitz]

Alternatively an approach could be to check the value of get_option before the early return. This way if there was a value stuck in persistent object cache we would still perform the db delete and wp_cache_* calls.

if ( is_null( $row ) && false === get_option( $option ) ) {
   return false;
}

This PR fixes the issue with delete_option failing to delete the cache if the option no longer exist in the database (deleted manually).

Trac ticket: https://core.trac.wordpress.org/ticket/52798

[donmhico]

I attached a PR above that should fix the issue. In the PR I moved the block of code which deletes the cache before it checks if the option exists in the database. I think that since we are deleting the option in DB, it's safe to delete it in the cache regardless of whether the option exists in database or not. Also you can notice that if the option is not present in the DB we are still returning false early, without triggering the delete_option and other actions. It's because I believe that those actions should only be triggered if the option is deleted in the database and shouldn't be triggered if the option are only deleted in the cache.

Any other thoughts / insights are welcome!

[desrosj]

Looks like this one was introduced in #3726 through [4855].