Transient with expiration never expires when storing expiration failed

[mennoll]

When storing a transient that has a timeout with the set_transient() function, there is a possibility that storing the transient timeout will fail. When you then try to get the transient with the get_transient() function, WordPress just returns it, which basically makes it behave like a transient with no expiration.

You can simulate the behaviour using this code:

<?php
// First store the transient
$transient_name       = 'testname';
$transient_value      = 'testvalue';
$transient_expiration = 30;
set_transient( $transient_name, $transient_value, $transient_expiration );

// Now delete the transient timeout, simulating the timeout option save failed
delete_option( '_transient_timeout_' . $transient_name );

// Now get the transient value
var_dump( get_transient( $transient_name ) ); // Returns 'testvalue'

To solve this issue, we need to modify the part in the get_transient() function, that checks if a transient is expired.

The new code of the get_transient() should become this: (see // <=== This is the changed line for what's changed.)

<?php
function get_transient( $transient ) {
 
    /**
     * Filters the value of an existing transient before it is retrieved.
     *
     * The dynamic portion of the hook name, `$transient`, refers to the transient name.
     *
     * Returning a truthy value from the filter will effectively short-circuit retrieval
     * and return the passed value instead.
     *
     * @since 2.8.0
     * @since 4.4.0 The `$transient` parameter was added
     *
     * @param mixed  $pre_transient The default value to return if the transient does not exist.
     *                              Any value other than false will short-circuit the retrieval
     *                              of the transient, and return that value.
     * @param string $transient     Transient name.
     */
    $pre = apply_filters( "pre_transient_{$transient}", false, $transient );
 
    if ( false !== $pre ) {
        return $pre;
    }
 
    if ( wp_using_ext_object_cache() ) {
        $value = wp_cache_get( $transient, 'transient' );
    } else {
        $transient_option = '_transient_' . $transient;
        if ( ! wp_installing() ) {
            // If option is not in alloptions, it is not autoloaded and thus has a timeout.
            $alloptions = wp_load_alloptions();
            if ( ! isset( $alloptions[ $transient_option ] ) ) {
                $transient_timeout = '_transient_timeout_' . $transient;
                $timeout           = get_option( $transient_timeout );
                if ( false === $timeout || ( false !== $timeout && $timeout < time() ) ) { // <=== This is the changed line
                    delete_option( $transient_option );
                    delete_option( $transient_timeout );
                    $value = false;
                }
            }
        }
 
        if ( ! isset( $value ) ) {
            $value = get_option( $transient_option );
        }
    }
 
    /**
     * Filters an existing transient's value.
     *
     * The dynamic portion of the hook name, `$transient`, refers to the transient name.
     *
     * @since 2.8.0
     * @since 4.4.0 The `$transient` parameter was added
     *
     * @param mixed  $value     Value of transient.
     * @param string $transient Transient name.
     */
    return apply_filters( "transient_{$transient}", $value, $transient );
}

When getting an existing transient that should have a timeout, but in reality it has not, delete the existing transient and return false.

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

[mennoll]

PR created:
​https://github.com/WordPress/wordpress-develop/pull/910

[peterwilsoncc]

For transients set without a timeout (ie, the $expriation parameter in set_transient() is zero or another falsy value) the timeout option is not set. This means the above approach would end up deleting permanent transients when getting them.

For expiring transients, an alternative approach would be a similar approach in set_transient(): if the transient value option is saved successfully but the timeout option fails to save then the value is deleted.

In very simplified code:

<?php
$timeout_result = add_option( $transient_timeout, time() + $expiration, '', 'no' );
if ( $timeout_result ) {
    $result = add_option( $transient_option, $value, '', 'no' );
    if ( ! $result ) {
        // delete both options.
    }
}

Due to the way set_transient() works, the simplified code would need to be repeated a couple of times and some modification for the calls to update_option() as it will return false if the new value matches the old.

This seems like a pretty edge-case bug so I am not sure if the additional code provides much benefit, is it something you have hit frequently?

It looks like this has been the case since transients where first introduced in 2.8, so I've updated the version to reflect that.

[mennoll]

Hi Peter, thank you for the response. To first reply to the last question you asked: yes, i've encountered this issue a few times now, in multiple websites. From testing, it looks like getting transients is going well, except for when a transient is not requested for some time. It looks like some garbage collection feature is deleting the transient timeout value from the database. I was not able to see where that went wrong, so I was not able to detect if that's a problem in WordPress Core, or in a plugin. I searched in my plugins but there was no code to be found that could interfear with how transiens work. Therefore it could be in core, but looking in code, I could not find the immediate problem.

However, like you can see, I was able to modify the get_transient() function to let it work correctly.

I see you are worried about that my solution might break transients that don't have a timeout. But I do think that this is not true. The code change I made is inside the if statement, that checks if a transient should have a timeout: (see this code below). Therefore I think this change only applies to transients that do have a timeout.

<?php
// If option is not in alloptions, it is not autoloaded and thus has a timeout.
$alloptions = wp_load_alloptions();
if ( ! isset( $alloptions[ $transient_option ] ) ) {
    // Code is here
}

I am indeed high aware of the issue that you pointed out in the Github PR, and how we could possibly fix my experienced bug via the get_transient() function. Unfortunately, I do also think that's not the full solution. I do think, we actually need both your suggestion, and mine.
To be even more clear, I do think that if I read trac ticket WP#30380, that the ticket is actually outdated. I fully understand that they wanted to save a few queries here because they seemed useless. But I think their unit test is not valid anymore.
We can however still pass their unit test, and also have a solution. If I change my code so that the transient will not be deleted, but the get_transient() still returns false, all unit tests should succeed.

To summarize, I think we need to discuss a little about the correct solution, and am happy to do so. Could you please give your opinion about the following list of questions:? Thanks :)

• I think that my change does not affect permanent transients. Could you please explain how you think it will?
• What do you think about both having a check in get_transient(), and your suggestion in set_transient()?
• I have doubt if i think the trac ticket 30380 is still valid, and therefore would like to know if you think that in my code deleting the transient there is the way to go (which requires that unit test to be deleted)

Thank you, hope to hear from you soon.

[mennoll]

Hi @peterwilsoncc ,

I would like to let you know that I've implemented your suggested modifications.
And I've also changed the included unit test.
It now actually actively simulates the actual cause of the problem, which was the failure of a database query.

I hope to hear if this is more what you had in mind!
Thanks in advance.

[mennoll]

Hi all,

Last year I've changed my Pull Request dunging the contributor day at WordCamp Europe 2022 to implement the changes requested.
And in about two days, another contributor day is happening at WCEU 2023 :).

I however see that my PR has not been yet reviewed, and I would like to know if there are changes to be made still. If there are, I would like to make the changes during contributor day at the Core table, but otherwise I might want to see if I can help on another department as well. Please let me know if able ;).

Kind regards, Menno