Skip to content

Commit ae66c45

Browse files
committed
tests: add standalone reproduction for the pipeline issue (#628)
The three "at once" bulk methods open a pipeline that is never discarded when something goes wrong: - add_multiple_at_once() and set_multiple_at_once() open the pipeline and queue commands outside the try block, so an exception raised while queueing (e.g. from a redis_cache_expiration filter) escapes with the pipeline still open - all three catch blocks call handle_exception() and return without calling discard(), so a failing exec() leaves the pipeline open too The connection is then stuck in pipeline mode and answers every later command with the client object itself, which makes wp_cache_get() fatal in add_to_internal_cache() ("Trying to clone an uncloneable object"), wp_cache_get_multiple() fatal in array_combine(), and set/delete report false while the writes actually land in Redis. Reproduces identically on PhpRedis 6.3.0 and Relay 0.40.0: php tests/pipeline-repro.php php tests/pipeline-repro.php --client=relay
1 parent 7264384 commit ae66c45

1 file changed

Lines changed: 278 additions & 0 deletions

File tree

tests/pipeline-repro.php

Lines changed: 278 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
1+
<?php
2+
/**
3+
* Standalone reproduction for https://github.com/rhubarbgroup/redis-cache/issues/628
4+
*
5+
* The three "at once" methods open a pipeline that is never discarded when
6+
* something goes wrong, which leaves the connection stuck in pipeline mode.
7+
* Every command issued afterwards then returns the client object itself
8+
* instead of a value.
9+
*
10+
* Reproduces identically on PhpRedis and Relay -- it is a drop-in bug, not a
11+
* client bug.
12+
*
13+
* Usage (needs a Redis server on 127.0.0.1:6379):
14+
*
15+
* php tests/pipeline-repro.php
16+
* php tests/pipeline-repro.php --client=relay
17+
*
18+
* @package Rhubarb\RedisCache
19+
*/
20+
21+
// phpcs:disable
22+
23+
$client = 'phpredis';
24+
25+
foreach ( array_slice( $argv, 1 ) as $arg ) {
26+
if ( strpos( $arg, '--client=' ) === 0 ) {
27+
$client = substr( $arg, 9 );
28+
}
29+
}
30+
31+
// --------------------------------------------------------------------------
32+
// Minimal WordPress shim, just enough to boot includes/object-cache.php.
33+
// --------------------------------------------------------------------------
34+
35+
define( 'ABSPATH', __DIR__ . '/' );
36+
define( 'WP_CONTENT_DIR', __DIR__ );
37+
define( 'WP_REDIS_CLIENT', $client );
38+
define( 'WP_REDIS_HOST', getenv( 'REDIS_HOST' ) ?: '127.0.0.1' );
39+
define( 'WP_REDIS_PORT', (int) ( getenv( 'REDIS_PORT' ) ?: 6379 ) );
40+
define( 'WP_REDIS_PREFIX', 'issue628:' );
41+
define( 'WP_REDIS_GRACEFUL', true );
42+
43+
$GLOBALS['table_prefix'] = 'wp_';
44+
$GLOBALS['blog_id'] = 1;
45+
$GLOBALS['wp_filter'] = [];
46+
47+
function add_filter( $hook, $callback, $priority = 10, $accepted_args = 1 ) {
48+
$GLOBALS['wp_filter'][ $hook ][] = $callback;
49+
50+
return true;
51+
}
52+
53+
function add_action( $hook, $callback, $priority = 10, $accepted_args = 1 ) {
54+
return add_filter( $hook, $callback, $priority, $accepted_args );
55+
}
56+
57+
function has_filter( $hook, $callback = false ) {
58+
return ! empty( $GLOBALS['wp_filter'][ $hook ] );
59+
}
60+
61+
function remove_all_filters( $hook ) {
62+
unset( $GLOBALS['wp_filter'][ $hook ] );
63+
}
64+
65+
function apply_filters( $hook, $value, ...$args ) {
66+
foreach ( isset( $GLOBALS['wp_filter'][ $hook ] ) ? $GLOBALS['wp_filter'][ $hook ] : [] as $callback ) {
67+
$value = $callback( $value, ...$args );
68+
}
69+
70+
return $value;
71+
}
72+
73+
function do_action( $hook, ...$args ) {
74+
foreach ( isset( $GLOBALS['wp_filter'][ $hook ] ) ? $GLOBALS['wp_filter'][ $hook ] : [] as $callback ) {
75+
$callback( ...$args );
76+
}
77+
}
78+
79+
require_once dirname( __DIR__ ) . '/includes/object-cache.php';
80+
81+
// --------------------------------------------------------------------------
82+
// Helpers.
83+
// --------------------------------------------------------------------------
84+
85+
$redis_property = new ReflectionProperty( WP_Object_Cache::class, 'redis' );
86+
$redis_property->setAccessible( true );
87+
88+
function boot_cache() {
89+
return $GLOBALS['wp_object_cache'] = new WP_Object_Cache();
90+
}
91+
92+
function client_of( $cache ) {
93+
return $GLOBALS['redis_property']->getValue( $cache );
94+
}
95+
96+
function replace_client( $cache, $client ) {
97+
$GLOBALS['redis_property']->setValue( $cache, $client );
98+
}
99+
100+
/**
101+
* A connection stuck in pipeline mode answers every command with itself.
102+
*/
103+
function in_pipeline_mode( $client ) {
104+
return $client->echo( 'probe' ) === $client;
105+
}
106+
107+
function describe( $value ) {
108+
if ( is_object( $value ) ) {
109+
return get_class( $value ) . ' object';
110+
}
111+
112+
if ( is_array( $value ) ) {
113+
return '[' . implode( ', ', array_map( 'describe', $value ) ) . ']';
114+
}
115+
116+
return var_export( $value, true );
117+
}
118+
119+
function line( $label, $value ) {
120+
printf( " %-44s %s\n", $label, describe( $value ) );
121+
}
122+
123+
function attempt( $label, callable $callback ) {
124+
try {
125+
line( $label, $callback() );
126+
} catch ( Throwable $throwable ) {
127+
line( $label, get_class( $throwable ) . ': ' . $throwable->getMessage() );
128+
}
129+
}
130+
131+
/**
132+
* Wraps the real client and fails exec() the way a dropped connection or a
133+
* read timeout does, while recording whether discard() is ever called.
134+
*/
135+
class FailingExecClient
136+
{
137+
public $inner;
138+
public $exec_calls = 0;
139+
public $discard_calls = 0;
140+
141+
public function __construct( $inner ) {
142+
$this->inner = $inner;
143+
}
144+
145+
public function pipeline() {
146+
$this->inner->pipeline();
147+
148+
return $this;
149+
}
150+
151+
public function exec() {
152+
$this->exec_calls++;
153+
154+
throw new RuntimeException( 'read error on connection to ' . WP_REDIS_HOST );
155+
}
156+
157+
public function discard() {
158+
$this->discard_calls++;
159+
160+
return $this->inner->discard();
161+
}
162+
163+
public function __call( $method, $arguments ) {
164+
$result = $this->inner->{$method}( ...$arguments );
165+
166+
return $result === $this->inner ? $this : $result;
167+
}
168+
}
169+
170+
// --------------------------------------------------------------------------
171+
172+
$cache = boot_cache();
173+
174+
if ( ! $cache->redis_status() ) {
175+
fwrite( STDERR, "Could not connect to Redis.\n" );
176+
exit( 1 );
177+
}
178+
179+
printf( "Client: %s\n\n", $cache->diagnostics['client'] );
180+
181+
echo "1. Baseline -- the bulk methods work\n";
182+
$cache->flush();
183+
line( 'add_multiple([a, b])', $cache->add_multiple( [ 'a' => 1, 'b' => 2 ], 'g' ) );
184+
line( 'connection in pipeline mode?', in_pipeline_mode( client_of( $cache ) ) ? 'YES' : 'no' );
185+
186+
echo "\n2. exec() fails mid-pipeline -- discard() is never called\n";
187+
echo " (the catch blocks at object-cache.php:1270, 1510 and 2305 return\n";
188+
echo " without taking the connection back out of pipeline mode)\n\n";
189+
190+
foreach ( [ 'add_multiple', 'set_multiple', 'delete_multiple' ] as $method ) {
191+
$cache = boot_cache();
192+
$real = client_of( $cache );
193+
$stub = new FailingExecClient( $real );
194+
replace_client( $cache, $stub );
195+
196+
$payload = $method === 'delete_multiple' ? [ 'k1', 'k2' ] : [ 'k1' => 1, 'k2' => 2 ];
197+
198+
try {
199+
$cache->{$method}( $payload, 'g' );
200+
} catch ( Throwable $throwable ) {
201+
// Swallowed by handle_exception() already.
202+
}
203+
204+
printf(
205+
" %-19s exec() failed: %-3s discard() called: %-3s left in pipeline mode: %s\n",
206+
$method . '()',
207+
$stub->exec_calls ? 'yes' : 'no',
208+
$stub->discard_calls ? 'yes' : 'no',
209+
in_pipeline_mode( $real ) ? 'YES' : 'no'
210+
);
211+
212+
$real->exec();
213+
}
214+
215+
echo "\n3. An exception while queueing -- same result\n";
216+
echo " (in add_multiple_at_once() and set_multiple_at_once() the queueing\n";
217+
echo " loop sits outside the try block, so nothing catches this at all)\n\n";
218+
219+
$cache = boot_cache();
220+
$cache->flush();
221+
$cache->add_multiple( [ 'a' => 1, 'b' => 2 ], 'g' );
222+
223+
add_filter( 'redis_cache_expiration', function ( $expiration, $key ) {
224+
if ( $key === 'boom' ) {
225+
throw new RuntimeException( 'a third-party redis_cache_expiration filter threw' );
226+
}
227+
228+
return $expiration;
229+
}, 10, 4 );
230+
231+
attempt( 'add_multiple([ok, boom, later])', function () use ( $cache ) {
232+
return $cache->add_multiple( [ 'ok' => 1, 'boom' => 2, 'later' => 3 ], 'g' );
233+
} );
234+
235+
remove_all_filters( 'redis_cache_expiration' );
236+
237+
line( 'connection in pipeline mode?', in_pipeline_mode( client_of( $cache ) ) ? 'YES' : 'no' );
238+
239+
echo "\n4. Every cache operation after that point is wrong\n";
240+
line( 'redis_status()', $cache->redis_status() );
241+
$cache->flush_runtime();
242+
attempt( 'get("a", "g") expected 1', function () use ( $cache ) {
243+
return $cache->get( 'a', 'g' );
244+
} );
245+
$cache->flush_runtime();
246+
attempt( 'get_multiple([a, b], "g")', function () use ( $cache ) {
247+
return $cache->get_multiple( [ 'a', 'b' ], 'g' );
248+
} );
249+
attempt( 'set("c", 3, "g") expected true', function () use ( $cache ) {
250+
return $cache->set( 'c', 3, 'g' );
251+
} );
252+
attempt( 'delete("a", "g") expected true', function () use ( $cache ) {
253+
return $cache->delete( 'a', 'g' );
254+
} );
255+
attempt( 'add_multiple([x, y], "g")', function () use ( $cache ) {
256+
return $cache->add_multiple( [ 'x' => 1, 'y' => 2 ], 'g' );
257+
} );
258+
259+
echo "\n ...while the writes that reported false did reach Redis anyway:\n";
260+
261+
$observer = boot_cache();
262+
line( 'keys issue628:*', client_of( $observer )->keys( 'issue628:*' ) );
263+
264+
echo "\n5. Does the wedged connection survive into the next request?\n";
265+
echo " No -- on both clients the next request gets a clean connection, even\n";
266+
echo " though Relay's connect() is persistent by default. So the damage is\n";
267+
echo " confined to the request that triggered it.\n\n";
268+
269+
$first = boot_cache();
270+
client_of( $first )->pipeline();
271+
client_of( $first )->set( 'issue628:leak', 1 );
272+
unset( $first );
273+
274+
$second = boot_cache();
275+
line( 'new request, pipeline mode?', in_pipeline_mode( client_of( $second ) ) ? 'YES -- still broken' : 'no -- recovered' );
276+
attempt( 'new request, get("a", "g")', function () use ( $second ) {
277+
return $second->get( 'a', 'g' );
278+
} );

0 commit comments

Comments
 (0)