From d10449f17d90f0ef4569bc0d6f0ca0ab2111319e Mon Sep 17 00:00:00 2001 From: Igor Tron Date: Sun, 5 May 2024 23:30:24 +0400 Subject: [PATCH 1/2] ghosts tests --- tests/lock/generic.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/lock/generic.php b/tests/lock/generic.php index 0809524..5b66164 100644 --- a/tests/lock/generic.php +++ b/tests/lock/generic.php @@ -238,6 +238,35 @@ public function test_concurrency_pageviews() { $wpdb->suppress_errors( $suppress_errors ); } + public function test_ghosts( $expiration = 0 ) { + $resource_id = $this->generate_lock_resource_id(); + $pid = getmypid(); + $children = []; + $callback = new WP_Lock_Backend_Callback( + function( $resource_id, $expiration ) { + $lock_backend = new WP_Lock_Backend_DB(); + // Born a ghost. + return $lock_backend->acquire( $resource_id, WP_Lock::WRITE, false, $expiration ); + }, + [ $resource_id, $expiration ] + ); + + $lock_backend = new WP_Lock_Backend_DB(); + + $children []= run_in_child( array( $callback, 'run' ) ); + foreach ( $children as $child ) { + pcntl_waitpid( $child, $status ); + $this->assertEmpty( pcntl_wexitstatus($status), "Unexpected exit code in _test_concurrency_pageviews_child PID $child"); + } + + $this->assertNotEmpty( $lock_backend->get_ghosts( $resource_id ) XOR $expiration, "No ghosts found for resourceID $resource_id, expiration $expiration, PID $pid" ); + $this->assertTrue( $lock_backend->acquire( $resource_id, WP_Lock::WRITE, false, 0 ) XOR $expiration ); + $this->assertEmpty( $lock_backend->get_ghosts( $resource_id ) ); + $lock_backend->release( $resource_id ); + + ++$expiration < 5 && $this->test_ghosts( $expiration ); + } + public function _test_concurrency_pageviews_child( $post_id, $resource_id, $lock_backend_class) { foreach ( range( 1, 100 ) as $_ ) { $this->_test_concurrency_pageviews_increment_counter( $post_id, $resource_id, $lock_backend_class ); From f5f3e239e2f9d22dab48106b3d1e11b1d75a7f36 Mon Sep 17 00:00:00 2001 From: Igor Tron Date: Sun, 5 May 2024 23:31:21 +0400 Subject: [PATCH 2/2] ghosts managing got optimized --- lib/backend/class-wp-lock-backend-db.php | 92 +++++++++++++++--------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/lib/backend/class-wp-lock-backend-db.php b/lib/backend/class-wp-lock-backend-db.php index c6b3a11..f383cee 100644 --- a/lib/backend/class-wp-lock-backend-db.php +++ b/lib/backend/class-wp-lock-backend-db.php @@ -18,12 +18,8 @@ class WP_Lock_Backend_DB implements WP_Lock_Backend { * Lock backend constructor. */ public function __construct() { - register_shutdown_function( function ( $lock ) { - /** - * Always try to unlock all storages when exiting. - */ - array_map( [ $lock, 'release' ], array_filter( $lock->lock_ids ) ); - }, $this ); + // Drop ghost locks. + $this->drop_ghosts(); } /** @@ -50,45 +46,73 @@ private function get_lock_key( string $id ): string { return md5( $id ); } + public function drop_ghosts( $lock_id = null ): bool { + global $wpdb; + $ghosts = $this->get_ghosts( $lock_id ); + + if ( ! empty( $ghosts ) ) { + $wpdb->query( "DELETE FROM {$this->get_table_name()} WHERE id IN (" . implode( ',', array_column( $ghosts, 'id' ) ) . ")" ); + return true; + } + + return false; + } + /** * Ghost lock is a lock that has no corresponding process and/or connection and has no expiration time. * Search for a ghost lock for specific lock_id in the database and remove it. * - * @param $lock_id - * - * @return bool + * @return array List of ghost locks. */ - public function drop_ghosts( $lock_id ): bool { + public function get_ghosts( $lock_id = null ): array { global $wpdb; - $lock_key = $this->get_lock_key( $lock_id ); - - $locks = $wpdb->get_results( "SELECT * FROM {$this->get_table_name()} WHERE `lock_key` = '$lock_key'", ARRAY_A ); - $ghosts = []; - foreach ( $locks as $lock ) { - if ( ! empty( $lock['expire'] ) && $lock['expire'] > microtime( true ) ) { - // This is an unexpired lock, keep it actual. No matter if it's a ghost or not. - continue; - } - if ( - ( empty( $lock['pid'] ) && empty( $lock['cid'] ) ) || - ! ( - ( ! empty( $lock['pid'] ) && file_exists( "/proc/{$lock['pid']}" ) ) || - ( ! empty( $lock['cid'] ) && $wpdb->get_var( $wpdb->prepare( "SELECT id FROM information_schema.processlist WHERE id = %s", $lock['cid'] ) ) ) - ) - ) { - // This is a ghost lock, remove it. - $ghosts[] = $lock['id']; - } + // Get all expired locks if no lock_id is provided. + $ids = $lock_id ? $wpdb->prepare( " AND `lock_key` = %s", $this->get_lock_key( $lock_id ) ) : ''; + + $expired = $wpdb->get_results( + $wpdb->prepare( + "SELECT * FROM {$this->get_table_name()} WHERE 1=1 {$ids} AND `expire` <= %f", + microtime( true ) + ), + ARRAY_A + ); + + if ( empty( $expired ) ) { + return []; } - if ( ! empty( $ghosts ) ) { - $wpdb->query( "DELETE FROM {$this->get_table_name()} WHERE id IN (" . implode( ',', $ghosts ) . ")" ); + // Following code supposes that there might be active locks with expiration field set 0. + // Filter out locks that have a corresponding process. They are not ghosts. + $expired = array_filter( $expired, function ( $lock ) { + return ! ( empty( $lock['expire'] ) && ! empty( $lock['pid'] ) && file_exists( "/proc/{$lock['pid']}" ) ); + } ); - return true; + if ( empty( $expired ) ) { + return []; } - return false; + $cids = array_column( $expired, 'cid' ); + if ( empty( $cids ) ) { + // Here we have only locks with no process ID and no connection ID. They are certainly ghosts. + return $expired; + } + + // Get active CIDs from the database to check whether the given connections are still alive or not. + $active_cids = $wpdb->get_col( + "SELECT id FROM information_schema.processlist WHERE id IN (" . implode( ',', $cids ) . ")" + ); + + $ghosts = array_filter( $expired, function ( $lock ) use ( $active_cids ) { + // Throw out locks that have a corresponding connection. They are not ghosts. + return ! ( + empty( $lock['expire'] ) && + ! empty( $lock['cid'] ) && + in_array( $lock['cid'], $active_cids, true ) + ); + } ); + + return $ghosts; } /** @@ -173,7 +197,7 @@ public function release( $id ) { $lock_id = $this->lock_ids[ $lock_key ]; unset( $this->lock_ids[ $lock_key ] ); - $wpdb->query( $wpdb->prepare( "DELETE FROM {$this->get_table_name()} WHERE id = %s", $lock_id ) ); + $wpdb->query( $wpdb->prepare( "DELETE FROM {$this->get_table_name()} WHERE id = %d", $lock_id ) ); return true; }