sem_release but no sem_remove #85

Closed
stoicbaby opened this Issue Dec 18, 2015 · 2 comments

Projects

None yet

3 participants

@stoicbaby

I work with some web hosting companies and we've been seeing a high volume of abandoned semaphores from our users from use of this plugin.

The exit function in wp-super-cache/wp-cache-phase2.php has no sem_remove once the semaphore has been released. Lines 201-219 are what I'm referring to in the file:

function wp_cache_writers_exit() {
    global $mutex, $wp_cache_mutex_disabled, $use_flock;

    if( isset( $wp_cache_mutex_disabled ) && $wp_cache_mutex_disabled )
            return true;

    if( !$mutex ) {
            wp_cache_debug( "(writers exit) mutex lock not created. not caching.", 2 );
            return false;
    }

    if ( $use_flock ) {
            wp_cache_debug( "releasing lock using flock()", 5 );
            flock($mutex,  LOCK_UN);
    } else {
            wp_cache_debug( "releasing lock using sem_release()", 5 );
            sem_release($mutex);
    }
}

Since this is the only exit function for the plugin, the semaphore should be removed within it. I understand that entirely removing the semaphores like this isn't the most efficient use of them, however the abandoned semaphores are creating apr_thread_request issues on apache. This issue has caused decreased apache performance and is affecting a higher volume of our user base which is something we'd like to avoid in the future.

My proposed fix is a semaphores patch to be rolled out as an update ASAP with the modified exit function:

function wp_cache_writers_exit() {
    global $mutex, $wp_cache_mutex_disabled, $use_flock;

    if( isset( $wp_cache_mutex_disabled ) && $wp_cache_mutex_disabled )
            return true;

    if( !$mutex ) {
            wp_cache_debug( "(writers exit) mutex lock not created. not caching.", 2 );
            return false;
    }

    if ( $use_flock ) {
            wp_cache_debug( "releasing lock using flock()", 5 );
            flock($mutex,  LOCK_UN);
    } else {
            wp_cache_debug( "releasing lock using sem_release()", 5 );
            sem_release($mutex);
            sem_remove($mutex);
    }
}

Unfortunately if this issue persists we may take action to disable the plugin on our servers to prevent further server wide performance issues.

@kraftbj kraftbj closed this in 8363947 Mar 7, 2016
@gregarios
gregarios commented May 17, 2016 edited

Why wasn't this included in the latest updates? Seems this problem still exists as my wp-cache-phase2.php file shows in version 1.4.8.

@donnchawp
Contributor

Do you advise your users use "coarse file locking"? That's disabled by default and not really needed any more because the plugin writes to filenames with random names before moving the data to their final cache file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment