diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 1ea1c0f..ef588bc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -28,6 +28,18 @@ jobs: name: PHP ${{ matrix.php }} - Laravel ${{ matrix.laravel }} + # The Redis pool and coroutine Redis tests skip without a server on 127.0.0.1:6379. + services: + redis: + image: redis:7-alpine + ports: + - 6379:6379 + options: >- + --health-cmd "redis-cli ping" + --health-interval 2s + --health-timeout 2s + --health-retries 15 + steps: - name: Checkout uses: actions/checkout@v4 @@ -36,7 +48,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: "${{ matrix.php }}" - extensions: swoole + extensions: swoole, redis coverage: none tools: composer:v2 cache: composer diff --git a/src/Swoole/Coroutine/CoroutineApplication.php b/src/Swoole/Coroutine/CoroutineApplication.php index c69f96a..90bb487 100644 --- a/src/Swoole/Coroutine/CoroutineApplication.php +++ b/src/Swoole/Coroutine/CoroutineApplication.php @@ -1272,20 +1272,51 @@ public function version() * @param callable|string $callback * @return $this */ + /** + * Register a terminating callback. + * + * A callback registered during a request belongs to that request. Forwarded to the + * base application it ran at the end of every later request and was never removed: + * the Blade engine, which Octane resolves again on every request, registers one each + * time, so a worker ran and kept one more callback per request it had served. + * + * @param callable|string $callback + * @return $this + */ public function terminating($callback) { + $scope = $this->getRequestScope(); + + if ($scope instanceof RequestScope) { + $scope->addTerminatingCallback($callback); + + return $this; + } + $this->getCurrentApp()->terminating($callback); + return $this; } /** * Terminate the application. * + * Runs the callbacks registered at boot, then the ones this request registered. + * * @return void */ public function terminate() { $this->getCurrentApp()->terminate(); + + $scope = $this->getRequestScope(); + + if ($scope instanceof RequestScope) { + // A callback may register another one; it runs in the same pass. + for ($index = 0; ($callback = $scope->terminatingCallback($index)) !== null; $index++) { + $this->call($callback); + } + } } /** @@ -1694,14 +1725,67 @@ public function provideFacades($namespace) return $this->getCurrentApp()->provideFacades($namespace); } + /** + * Register a rebinding callback. + * + * A rebinding of a request-scoped abstract belongs to the request. Laravel's + * AuthManager calls refresh('request', $guard, 'setRequest') for every guard + * it creates, and the auth manager is request-scoped, so delegating to the + * base application kept one callback (and the guard, request, session and + * user it captures) per request for the life of the worker. It also handed + * the guard the base application's request instead of the current one. + * + * @param string $abstract + * @param \Closure $callback + * @return mixed + */ public function rebinding($abstract, \Closure $callback) { + $scope = $this->getRequestScope(); + + if ($scope instanceof RequestScope + && is_string($key = $this->normalizeAbstract($abstract)) + && $scope->has($key)) { + $scope->addReboundCallback($key, $callback); + + return $this->make($key); + } + return $this->getCurrentApp()->rebinding($abstract, $callback); } public function refresh($abstract, $target, $method) { - return $this->getCurrentApp()->refresh($abstract, $target, $method); + return $this->rebinding($abstract, function ($app, $instance) use ($target, $method) { + $target->{$method}($instance); + }); + } + + /** + * Fire the "rebound" callbacks for the given abstract type. + * + * Request-local callbacks run first, then any registered on this proxy. + * + * @param string $abstract + * @return void + */ + protected function rebound($abstract) + { + $scope = $this->getRequestScope(); + + if ($scope instanceof RequestScope && is_string($key = $this->normalizeAbstract($abstract))) { + $callbacks = $scope->reboundCallbacks($key); + + if ($callbacks !== []) { + $instance = $this->make($key); + + foreach ($callbacks as $callback) { + $callback($this, $instance); + } + } + } + + parent::rebound($abstract); } public function registerConfiguredProviders() diff --git a/src/Swoole/Coroutine/RequestScope.php b/src/Swoole/Coroutine/RequestScope.php index 56cb386..50cb3b5 100644 --- a/src/Swoole/Coroutine/RequestScope.php +++ b/src/Swoole/Coroutine/RequestScope.php @@ -52,6 +52,20 @@ class RequestScope */ private array $resolvedBindings = []; + /** + * Rebinding callbacks registered for request-scoped abstracts. + * + * @var array> + */ + private array $reboundCallbacks = []; + + /** + * Callbacks to run when this request terminates. + * + * @var list + */ + private array $terminatingCallbacks = []; + /** * The base application instance (shared, read-only reference). * @@ -136,6 +150,51 @@ public function forget(string $key): void unset($this->bindings[$key], $this->bindingDefinitions[$key], $this->resolvedBindings[$key]); } + /** + * Register a rebinding callback that lives as long as this request. + * + * @param string $key + * @param \Closure $callback + * @return void + */ + public function addReboundCallback(string $key, Closure $callback): void + { + $this->reboundCallbacks[$key][] = $callback; + } + + /** + * Register a callback to run when this request terminates. + * + * @param callable|string $callback + * @return void + */ + public function addTerminatingCallback($callback): void + { + $this->terminatingCallbacks[] = $callback; + } + + /** + * Get the request's terminating callback at the given position, if any. + * + * @param int $index + * @return callable|string|null + */ + public function terminatingCallback(int $index) + { + return $this->terminatingCallbacks[$index] ?? null; + } + + /** + * Get the request-local rebinding callbacks for the given abstract. + * + * @param string $key + * @return list<\Closure> + */ + public function reboundCallbacks(string $key): array + { + return $this->reboundCallbacks[$key] ?? []; + } + /** * Register a request-local binding. * @@ -288,6 +347,7 @@ protected function resolveSpecializedBinding(string $key, Application $sandbox) return match ($key) { 'auth' => $this->createAuthManager($sandbox), + \Illuminate\Bus\Dispatcher::class => $this->createBusDispatcher($sandbox), 'auth.driver' => $this->createAuthDriver($sandbox), 'cache', \Illuminate\Contracts\Cache\Factory::class => $this->createCacheManager($sandbox), 'cache.store', \Illuminate\Contracts\Cache\Repository::class => $this->createCacheStore($sandbox), @@ -713,6 +773,8 @@ public function clear(): void $this->bindings = []; $this->bindingDefinitions = []; $this->resolvedBindings = []; + $this->reboundCallbacks = []; + $this->terminatingCallbacks = []; $this->configCloned = false; } @@ -1394,6 +1456,33 @@ protected function createInertiaResponseFactory(): \Inertia\ResponseFactory return new \Inertia\ResponseFactory; } + /** + * Create a bus dispatcher bound to the current request. + * + * dispatch(...)->afterResponse() and Bus::batch() register a terminating callback on + * the dispatcher's container. On the worker's singleton that is the base application, + * whose terminating list is never cleared and runs at the end of every request: each + * after-response job ran again at the end of every later request on the worker. A + * clone holding the request's container keeps the callback with this request. A fake + * (Bus::fake()) is not a Dispatcher and stays shared. + * + * @param \Illuminate\Foundation\Application $sandbox + * @return \Illuminate\Bus\Dispatcher|null + */ + protected function createBusDispatcher(Application $sandbox): ?\Illuminate\Bus\Dispatcher + { + $dispatcher = $this->app->make(\Illuminate\Bus\Dispatcher::class); + + if (! $dispatcher instanceof \Illuminate\Bus\Dispatcher) { + return null; + } + + $dispatcher = clone $dispatcher; + $this->setObjectProperty($dispatcher, 'container', $sandbox); + + return $dispatcher; + } + /** * Create an isolated Vite instance for the current coroutine. * diff --git a/src/Swoole/Redis/CoroutineRedisManager.php b/src/Swoole/Redis/CoroutineRedisManager.php index be3e0e0..0bbad8e 100644 --- a/src/Swoole/Redis/CoroutineRedisManager.php +++ b/src/Swoole/Redis/CoroutineRedisManager.php @@ -26,7 +26,9 @@ * manager coroutine-aware closes that hole for every such holder, because * RedisStore re-resolves the connection on each operation. * - * Connections are released by the worker when the request coroutine finishes. + * Connections are released by the worker when the request coroutine finishes. With + * `octane.swoole.redis_pool.enabled`, clean phpredis connections go back to a per-worker + * pool (see RedisConnectionPool) instead of being closed. */ class CoroutineRedisManager extends BaseRedisManager { @@ -55,6 +57,20 @@ class CoroutineRedisManager extends BaseRedisManager */ protected $coroutineResolver = null; + /** + * The worker's Redis connection pool, shared by every manager and clone in the process. + * + * @var \Laravel\Octane\Swoole\Redis\RedisConnectionPool|null + */ + protected static ?RedisConnectionPool $pool = null; + + /** + * Whether the pool configuration has been read in this process. + * + * @var bool + */ + protected static bool $poolConfigured = false; + /** * Get a Redis connection by name. * @@ -75,39 +91,63 @@ public function connection($name = null) return $connection; } - $connection = $this->configure( - $this->coroutineResolver()->resolve($name), $name - ); + $poolKey = $this->pool() !== null ? $this->poolKey($name) : null; + $pool = $poolKey !== null ? $this->pool() : null; + + $connection = $pool?->borrow($poolKey); + + if ($connection === null) { + $connection = $this->coroutineResolver()->resolve($name); + + if ($pool !== null && $connection instanceof PooledPhpRedisConnection) { + $pool->track($connection); + } + } + + $connection = $this->configure($connection, $name); Context::set($key, $connection); $names = Context::get(self::CONTEXT_NAMES, []); - $names[$name] = true; + $names[$name] = $poolKey ?? true; Context::set(self::CONTEXT_NAMES, $names); return $connection; } /** - * Close and forget the connections opened by the current coroutine. + * Close (or return to the pool) and forget the connections opened by the current coroutine. * * @return int */ public function releaseConnections(): int + { + return $this->releaseDetachedConnections($this->detachConnections()); + } + + /** + * Take the current coroutine's connections out of its context without closing them. + * + * The worker detaches them before the coroutine context is cleared and releases them + * after the request has been torn down, so nothing from the request can still use a + * socket that another request has borrowed from the pool. + * + * @return list + */ + public function detachConnections(): array { if (! Context::inCoroutine()) { - return 0; + return []; } - $released = 0; + $detached = []; - foreach (array_keys(Context::get(self::CONTEXT_NAMES, [])) as $name) { + foreach (Context::get(self::CONTEXT_NAMES, []) as $name => $poolKey) { $key = self::CONTEXT_PREFIX.$name; $connection = Context::get($key); if ($connection instanceof Connection) { - $this->disconnectConnection($connection); - $released++; + $detached[] = [$connection, $poolKey]; } Context::delete($key); @@ -115,7 +155,31 @@ public function releaseConnections(): int Context::delete(self::CONTEXT_NAMES); - return $released; + return $detached; + } + + /** + * Return detached connections to the pool, closing any the pool will not take. + * + * @param list $detached + * @return int + */ + public function releaseDetachedConnections(array $detached): int + { + $pool = $this->pool(); + + foreach ($detached as [$connection, $poolKey]) { + $pooled = $pool !== null + && is_string($poolKey) + && $connection instanceof PooledPhpRedisConnection + && $pool->release($poolKey, $connection); + + if (! $pooled) { + $this->disconnectConnection($connection); + } + } + + return count($detached); } /** @@ -185,10 +249,12 @@ public function setDriver($driver) protected function coroutineResolver(): BaseRedisManager { if ($this->coroutineResolver === null) { - $resolver = new BaseRedisManager( + $resolver = new CoroutineRedisResolver( $this->app, $this->driver, $this->coroutineSafeConfig() ); + $resolver->pooled = $this->pool() !== null; + foreach ($this->customCreators as $driver => $creator) { $resolver->extend($driver, $creator); } @@ -199,6 +265,64 @@ protected function coroutineResolver(): BaseRedisManager return $this->coroutineResolver; } + /** + * Get the worker's Redis pool, or null when pooling is disabled. + * + * @return \Laravel\Octane\Swoole\Redis\RedisConnectionPool|null + */ + protected function pool(): ?RedisConnectionPool + { + if (! static::$poolConfigured) { + $config = []; + + try { + $config = (array) $this->app->make('config')->get('octane.swoole.redis_pool', []); + } catch (Throwable) { + // No configuration: pooling stays off. + } + + static::$pool = filter_var($config['enabled'] ?? false, FILTER_VALIDATE_BOOLEAN) + ? RedisConnectionPool::fromConfig($config) + : null; + + static::$poolConfigured = true; + } + + return static::$pool; + } + + /** + * Forget the worker's Redis pool, closing its idle connections. + * + * @return void + */ + public static function flushPool(): void + { + static::$pool?->flush(); + static::$pool = null; + static::$poolConfigured = false; + } + + /** + * Identify the connection settings a pooled connection was opened with. + * + * @param string $name + * @return string + */ + protected function poolKey(string $name): ?string + { + try { + return $name.'|'.md5(serialize([ + $this->driver, + $this->config[$name] ?? null, + $this->config['options'] ?? null, + ])); + } catch (Throwable) { + // A configuration that cannot be serialized (e.g. holding a closure) is not pooled. + return null; + } + } + /** * Build a configuration with persistent connections disabled. * diff --git a/src/Swoole/Redis/CoroutineRedisResolver.php b/src/Swoole/Redis/CoroutineRedisResolver.php new file mode 100644 index 0000000..6096856 --- /dev/null +++ b/src/Swoole/Redis/CoroutineRedisResolver.php @@ -0,0 +1,34 @@ +pooled && $this->driver === 'phpredis' && ! isset($this->customCreators['phpredis'])) { + return new PooledPhpRedisConnector; + } + + return parent::connector(); + } +} diff --git a/src/Swoole/Redis/PooledPhpRedisConnection.php b/src/Swoole/Redis/PooledPhpRedisConnection.php new file mode 100644 index 0000000..dcc2b5c --- /dev/null +++ b/src/Swoole/Redis/PooledPhpRedisConnection.php @@ -0,0 +1,153 @@ + + */ + protected const STATEFUL_COMMANDS = [ + 'auth', 'client', 'hello', 'monitor', 'psubscribe', 'punsubscribe', 'readonly', + 'readwrite', 'reset', 'select', 'setoption', 'ssubscribe', 'subscribe', 'sunsubscribe', + 'swapdb', 'unsubscribe', 'watch', + ]; + + /** + * Commands that are only unsafe when sent raw, because phpredis does not track them. + * + * @var list + */ + protected const STATEFUL_RAW_COMMANDS = ['discard', 'exec', 'multi']; + + /** + * Whether nothing has made the connection unsafe to share. + * + * @var bool + */ + protected bool $reusable = true; + + /** + * The number of commands that started but did not return. + * + * @var int + */ + protected int $unfinishedCommands = 0; + + /** + * Wrap a connection created by the framework connector. + * + * @param \Illuminate\Redis\Connections\PhpRedisConnection $connection + * @return static + */ + public static function fromConnection(PhpRedisConnection $connection): static + { + return new static($connection->client, $connection->connector, $connection->config); + } + + /** + * Run a command against the Redis database. + * + * @param string $method + * @param array $parameters + * @return mixed + */ + public function command($method, array $parameters = []) + { + $name = strtolower($method); + + if (in_array($name, static::STATEFUL_COMMANDS, true)) { + $this->reusable = false; + } elseif ($name === 'rawcommand') { + $raw = is_string($parameters[0] ?? null) ? strtolower($parameters[0]) : ''; + + if (in_array($raw, static::STATEFUL_COMMANDS, true) || in_array($raw, static::STATEFUL_RAW_COMMANDS, true)) { + $this->reusable = false; + } + } + + $this->unfinishedCommands++; + + $result = parent::command($method, $parameters); + + $this->unfinishedCommands--; + + return $result; + } + + /** + * Subscribe to a set of given channels with wildcards. + * + * @param array|string $channels + * @param \Closure $callback + * @param string $method + * @return void + */ + public function createSubscription($channels, Closure $callback, $method = 'subscribe') + { + $this->reusable = false; + + parent::createSubscription($channels, $callback, $method); + } + + /** + * Determine whether the connection can be handed to another request. + * + * @return bool + */ + public function canReturnToPool(): bool + { + return $this->reusable && $this->unfinishedCommands === 0; + } + + /** + * Get the callable that reconnects this connection. + * + * @return callable|null + */ + public function connector(): ?callable + { + return $this->connector; + } + + /** + * Get the connection configuration. + * + * @return array + */ + public function config(): array + { + return $this->config; + } + + /** + * Take the socket away from this lease; later use of the lease throws. + * + * @return mixed + */ + public function detachClient() + { + $client = $this->client; + + $this->client = new ReleasedRedisClient; + $this->connector = null; + $this->reusable = false; + + return $client; + } +} diff --git a/src/Swoole/Redis/PooledPhpRedisConnector.php b/src/Swoole/Redis/PooledPhpRedisConnector.php new file mode 100644 index 0000000..a7215c1 --- /dev/null +++ b/src/Swoole/Redis/PooledPhpRedisConnector.php @@ -0,0 +1,23 @@ +> + */ + protected array $idle = []; + + /** + * Creation time and state snapshot of the leases that are out. + * + * @var \WeakMap<\Laravel\Octane\Swoole\Redis\PooledPhpRedisConnection, array{0: float, 1: array}> + */ + protected WeakMap $loaned; + + /** + * The process the pool belongs to. + * + * @var int + */ + protected int $pid; + + /** + * Create a new pool. + * + * @param int $maxIdle Idle sockets kept per pool key. + * @param float $maxIdleTime Seconds an idle socket may wait before it is closed. + * @param float $pingAfterIdle Seconds idle after which a socket is pinged before reuse. + * @param float $maxLifetime Seconds after which a socket is closed instead of reused. + */ + public function __construct( + protected int $maxIdle = 16, + protected float $maxIdleTime = 60.0, + protected float $pingAfterIdle = 5.0, + protected float $maxLifetime = 300.0, + ) { + $this->loaned = new WeakMap; + $this->pid = getmypid(); + } + + /** + * Create a pool from the `octane.swoole.redis_pool` configuration. + * + * @param array $config + * @return static + */ + public static function fromConfig(array $config): static + { + return new static( + max(0, (int) ($config['max_idle'] ?? 16)), + (float) ($config['max_idle_time'] ?? 60), + (float) ($config['ping_after_idle'] ?? 5), + (float) ($config['max_lifetime'] ?? 300), + ); + } + + /** + * Lend an idle socket for the given key in a new lease, if a healthy one is available. + * + * @param string $key + * @return \Laravel\Octane\Swoole\Redis\PooledPhpRedisConnection|null + */ + public function borrow(string $key): ?PooledPhpRedisConnection + { + $this->forgetSocketsFromAnotherProcess(); + $this->pruneExpired($key); + + while (($entry = array_pop($this->idle[$key])) !== null) { + $now = microtime(true); + + if ($now - $entry['created'] >= $this->maxLifetime) { + $this->close($entry['client']); + + continue; + } + + if ($now - $entry['released'] >= $this->pingAfterIdle && ! $this->ping($entry['client'])) { + $this->close($entry['client']); + + continue; + } + + $lease = new PooledPhpRedisConnection($entry['client'], $entry['connector'], $entry['config']); + $this->loaned[$lease] = [$entry['created'], $entry['state']]; + + return $lease; + } + + return null; + } + + /** + * Start tracking a connection that was just opened. + * + * @param \Laravel\Octane\Swoole\Redis\PooledPhpRedisConnection $connection + * @return void + */ + public function track(PooledPhpRedisConnection $connection): void + { + if (($state = $this->state($connection->client())) !== null) { + $this->loaned[$connection] = [microtime(true), $state]; + } + } + + /** + * Take a socket back from its lease. Returns false when the caller must close it instead. + * + * @param string $key + * @param \Laravel\Octane\Swoole\Redis\PooledPhpRedisConnection $connection + * @return bool + */ + public function release(string $key, PooledPhpRedisConnection $connection): bool + { + $loan = $this->loaned[$connection] ?? null; + unset($this->loaned[$connection]); + + if ($loan === null || $this->pid !== getmypid()) { + return false; + } + + [$created, $state] = $loan; + $now = microtime(true); + + $this->pruneExpired($key); + + if (count($this->idle[$key] ?? []) >= $this->maxIdle + || $now - $created >= $this->maxLifetime + || ! $connection->canReturnToPool() + || $this->state($connection->client()) !== $state) { + return false; + } + + $connector = $connection->connector(); + $config = $connection->config(); + + $this->idle[$key][] = [ + 'client' => $connection->detachClient(), + 'connector' => $connector, + 'config' => $config, + 'created' => $created, + 'released' => $now, + 'state' => $state, + ]; + + return true; + } + + /** + * Close every idle socket. + * + * @return void + */ + public function flush(): void + { + foreach ($this->idle as $entries) { + foreach ($entries as $entry) { + $this->close($entry['client']); + } + } + + $this->idle = []; + } + + /** + * The number of idle sockets for the given key. + * + * @param string $key + * @return int + */ + public function idleCount(string $key): int + { + return count($this->idle[$key] ?? []); + } + + /** + * Close idle sockets that waited too long; the oldest releases sit at the bottom. + * + * @param string $key + * @return void + */ + protected function pruneExpired(string $key): void + { + $this->idle[$key] ??= []; + $now = microtime(true); + + while ($this->idle[$key] !== [] + && ($now - $this->idle[$key][0]['released'] >= $this->maxIdleTime + || $now - $this->idle[$key][0]['created'] >= $this->maxLifetime)) { + $this->close(array_shift($this->idle[$key])['client']); + } + } + + /** + * Read the socket state a clean connection must keep, or null if it is not usable. + * + * @param mixed $client + * @return array|null + */ + protected function state($client): ?array + { + try { + if (! $client instanceof Redis || ! $client->isConnected() || $client->getMode() !== Redis::ATOMIC) { + return null; + } + + $state = [$client->getDbNum()]; + + foreach ([ + 'OPT_PREFIX', 'OPT_SERIALIZER', 'OPT_COMPRESSION', 'OPT_READ_TIMEOUT', 'OPT_SCAN', + 'OPT_REPLY_LITERAL', 'OPT_NULL_MULTIBULK_AS_NULL', 'OPT_MAX_RETRIES', + ] as $option) { + if (defined(Redis::class.'::'.$option)) { + $state[$option] = $client->getOption(constant(Redis::class.'::'.$option)); + } + } + + return $state; + } catch (Throwable) { + return null; + } + } + + /** + * Check that an idle socket still answers. + * + * @param \Redis $client + * @return bool + */ + protected function ping($client): bool + { + try { + $reply = $client->ping(); + + return $reply === true || $reply === '+PONG' || $reply === 'PONG'; + } catch (Throwable) { + return false; + } + } + + /** + * Close a socket, ignoring transport errors. + * + * @param mixed $client + * @return void + */ + protected function close($client): void + { + try { + $client->close(); + } catch (Throwable) { + // The socket is being discarded either way. + } + } + + /** + * Drop sockets inherited across a fork; they belong to the parent process. + * + * @return void + */ + protected function forgetSocketsFromAnotherProcess(): void + { + if ($this->pid === getmypid()) { + return; + } + + $this->idle = []; + $this->loaned = new WeakMap; + $this->pid = getmypid(); + } +} diff --git a/src/Swoole/Redis/ReleasedRedisClient.php b/src/Swoole/Redis/ReleasedRedisClient.php new file mode 100644 index 0000000..043afb6 --- /dev/null +++ b/src/Swoole/Redis/ReleasedRedisClient.php @@ -0,0 +1,48 @@ +make(LivewireCoroutineMutex::class)->releaseAllForCurrentCoroutine(); } - $this->releaseCoroutineRedisConnections(); + // Detached before flush() (whose scope purge would close them) but + // released only after the request is torn down, in the finally + // block below, like the database connections: with the Redis pool + // on, a socket returned earlier could be borrowed by another request + // while this request's teardown still used it. + $redisConnections = $this->detachCoroutineRedisConnections(); } $sandbox->flush(); @@ -196,6 +202,7 @@ public function handle(Request $request, RequestContext $context): void } } finally { $this->releaseDetachedDatabaseConnections($pooledConnections); + $this->releaseDetachedRedisConnections($redisConnections); if ($inCoroutine) { // A destructor run during teardown or gc above may have @@ -225,24 +232,54 @@ public function handle(Request $request, RequestContext $context): void } /** - * Close the Redis connections the worker-level manager opened for this coroutine. + * Take the Redis connections the worker-level manager opened for this coroutine + * out of its context, to be released once the request has been torn down. * * The manager keeps them in coroutine context so concurrent requests never * share a hooked phpredis socket. Without this they would only be dropped * when the context is cleared, leaving the sockets open in the meantime. + * + * @return array{0?: object, 1?: array} */ - protected function releaseCoroutineRedisConnections(): void + protected function detachCoroutineRedisConnections(): array { try { if (! $this->app->resolved('redis')) { - return; + return []; } $redis = $this->app->make('redis'); + if (method_exists($redis, 'detachConnections')) { + return [$redis, $redis->detachConnections()]; + } + if (method_exists($redis, 'releaseConnections')) { $redis->releaseConnections(); } + } catch (Throwable $e) { + error_log('⚠️ Failed to detach coroutine Redis connections: '.$e->getMessage()); + } + + return []; + } + + /** + * Return the request's detached Redis connections to the pool, or close them. + * + * @param array $detached + * @return void + */ + protected function releaseDetachedRedisConnections(array $detached): void + { + if ($detached === []) { + return; + } + + [$redis, $connections] = $detached; + + try { + $redis->releaseDetachedConnections($connections); } catch (Throwable $e) { error_log('⚠️ Failed to release coroutine Redis connections: '.$e->getMessage()); } diff --git a/tests/Unit/CoroutineRebindingTest.php b/tests/Unit/CoroutineRebindingTest.php new file mode 100644 index 0000000..87387a2 --- /dev/null +++ b/tests/Unit/CoroutineRebindingTest.php @@ -0,0 +1,208 @@ +applications(); + $current = Request::create('/current'); + $target = new RequestRecorder; + + Context::set('octane.request_scope', new RequestScope($base, $current)); + + $this->assertSame($current, $sandbox->refresh('request', $target, 'setRequest')); + $this->assertSame([], $this->baseReboundCallbacks($base, 'request')); + } + + public function test_rebinding_the_request_inside_a_request_reaches_request_local_targets(): void + { + [$base, $sandbox] = $this->applications(); + $target = new RequestRecorder; + + Context::set('octane.request_scope', new RequestScope($base, Request::create('/first'))); + + $sandbox->refresh('request', $target, 'setRequest'); + $sandbox->instance('request', $replacement = Request::create('/replacement')); + + $this->assertSame([$replacement], $target->received); + } + + public function test_concurrent_requests_do_not_run_each_others_rebinding_callbacks(): void + { + if (! class_exists(Coroutine::class) || ! class_exists(Channel::class)) { + $this->markTestSkipped('Swoole coroutine support is required.'); + } + + [$base, $sandbox] = $this->applications(); + $targetA = new RequestRecorder; + $targetB = new RequestRecorder; + + Coroutine\run(function () use ($base, $sandbox, $targetA, $targetB) { + $aRegistered = new Channel(1); + $bRebound = new Channel(1); + $done = new Channel(2); + + Coroutine::create(function () use ($base, $sandbox, $targetA, $aRegistered, $bRebound, $done) { + Context::set('octane.request_scope', new RequestScope($base, Request::create('/a'))); + $sandbox->refresh('request', $targetA, 'setRequest'); + + $aRegistered->push(true); + $bRebound->pop(); + + $done->push(true); + }); + + Coroutine::create(function () use ($base, $sandbox, $targetB, $aRegistered, $bRebound, $done) { + $aRegistered->pop(); + + Context::set('octane.request_scope', new RequestScope($base, Request::create('/b'))); + $sandbox->refresh('request', $targetB, 'setRequest'); + $sandbox->instance('request', Request::create('/b-replacement')); + + $bRebound->push(true); + $done->push(true); + }); + + $done->pop(); + $done->pop(); + }); + + $this->assertSame([], $targetA->received); + $this->assertSame(['/b-replacement'], array_map(fn (Request $r) => $r->getPathInfo(), $targetB->received)); + $this->assertSame([], $this->baseReboundCallbacks($base, 'request')); + } + + public function test_outside_a_request_refresh_still_registers_on_the_base_application(): void + { + [$base, $sandbox] = $this->applications(); + $base->instance('request', $bootRequest = Request::create('/boot')); + + $this->assertSame($bootRequest, $sandbox->refresh('request', new RequestRecorder, 'setRequest')); + $this->assertCount(1, $this->baseReboundCallbacks($base, 'request')); + } + + public function test_the_session_guard_holds_the_current_request_and_leaves_no_callback_on_the_base(): void + { + [$base, $sandbox] = $this->applications(); + $base->instance('request', Request::create('/boot')); + $base->make('auth')->provider('stub', fn () => new StubUserProvider); + + $current = Request::create('/dashboard', 'GET', [], ['remember_web' => 'cookie-value']); + + Context::set('octane.request_scope', new RequestScope($base, $current)); + + $guard = $sandbox->make('auth')->guard('web'); + + $this->assertInstanceOf(SessionGuard::class, $guard); + $this->assertSame($current, $guard->getRequest()); + $this->assertSame([], $this->baseReboundCallbacks($base, 'request')); + } + + /** + * @return array{0: \Illuminate\Foundation\Application, 1: \Laravel\Octane\Swoole\Coroutine\CoroutineApplication} + */ + private function applications(): array + { + $base = new Application(__DIR__); + + $base->instance('config', new ConfigRepository([ + 'auth' => [ + 'defaults' => ['guard' => 'web'], + 'guards' => ['web' => ['driver' => 'session', 'provider' => 'users']], + 'providers' => ['users' => ['driver' => 'stub']], + ], + 'session' => ['driver' => 'array', 'cookie' => 'session'], + ])); + $base->instance('session', new SessionManager($base)); + $base->singleton('session.store', fn ($app) => $app->make('session')->driver()); + $base->instance('cookie', new CookieJar); + $base->instance('auth', new AuthManager($base)); + + return [$base, new CoroutineApplication($base)]; + } + + /** + * @return list<\Closure> + */ + private function baseReboundCallbacks(Application $base, string $abstract): array + { + return (new ReflectionProperty($base, 'reboundCallbacks'))->getValue($base)[$abstract] ?? []; + } +} + +class RequestRecorder +{ + /** + * @var list<\Illuminate\Http\Request> + */ + public array $received = []; + + public function setRequest(Request $request): void + { + $this->received[] = $request; + } +} + +class StubUserProvider implements UserProvider +{ + public function retrieveById($identifier): ?Authenticatable + { + return null; + } + + public function retrieveByToken($identifier, $token): ?Authenticatable + { + return null; + } + + public function updateRememberToken(Authenticatable $user, $token): void + { + } + + public function retrieveByCredentials(array $credentials): ?Authenticatable + { + return null; + } + + public function validateCredentials(Authenticatable $user, array $credentials): bool + { + return false; + } + + public function rehashPasswordIfRequired(Authenticatable $user, array $credentials, bool $force = false): void + { + } +} diff --git a/tests/Unit/CoroutineTerminatingTest.php b/tests/Unit/CoroutineTerminatingTest.php new file mode 100644 index 0000000..b0ee0ef --- /dev/null +++ b/tests/Unit/CoroutineTerminatingTest.php @@ -0,0 +1,184 @@ +applications(); + $ran = []; + $base->terminating(function () use (&$ran) { + $ran[] = 'boot'; + }); + + Context::set('octane.request_scope', new RequestScope($base, Request::create('/one'))); + $sandbox->terminating(function () use (&$ran) { + $ran[] = 'one'; + }); + $sandbox->terminate(); + + $this->assertSame(['boot', 'one'], $ran); + $this->assertCount(1, $this->baseTerminatingCallbacks($base)); + } + + public function test_a_later_request_does_not_run_an_earlier_requests_callbacks(): void + { + [$base, $sandbox] = $this->applications(); + $ran = []; + + Context::set('octane.request_scope', $first = new RequestScope($base, Request::create('/first'))); + $sandbox->terminating(function () use (&$ran) { + $ran[] = 'first'; + }); + $sandbox->terminate(); + $first->clear(); + + Context::set('octane.request_scope', new RequestScope($base, Request::create('/second'))); + $sandbox->terminate(); + + $this->assertSame(['first'], $ran); + } + + public function test_a_callback_registered_while_terminating_runs_in_the_same_pass(): void + { + [$base, $sandbox] = $this->applications(); + $ran = []; + + Context::set('octane.request_scope', new RequestScope($base, Request::create('/nested'))); + $sandbox->terminating(function () use ($sandbox, &$ran) { + $ran[] = 'outer'; + $sandbox->terminating(function () use (&$ran) { + $ran[] = 'inner'; + }); + }); + $sandbox->terminate(); + + $this->assertSame(['outer', 'inner'], $ran); + } + + public function test_concurrent_requests_only_run_their_own_callbacks(): void + { + if (! class_exists(Coroutine::class) || ! class_exists(Channel::class)) { + $this->markTestSkipped('Swoole coroutine support is required.'); + } + + [$base, $sandbox] = $this->applications(); + $ran = ['a' => [], 'b' => []]; + + Coroutine\run(function () use ($base, $sandbox, &$ran) { + $aRegistered = new Channel(1); + $bTerminated = new Channel(1); + $done = new Channel(2); + + Coroutine::create(function () use ($base, $sandbox, &$ran, $aRegistered, $bTerminated, $done) { + Context::set('octane.request_scope', new RequestScope($base, Request::create('/a'))); + $sandbox->terminating(function () use (&$ran) { + $ran['a'][] = 'a'; + }); + $aRegistered->push(true); + $bTerminated->pop(); + $done->push(true); + }); + + Coroutine::create(function () use ($base, $sandbox, &$ran, $aRegistered, $bTerminated, $done) { + $aRegistered->pop(); + Context::set('octane.request_scope', new RequestScope($base, Request::create('/b'))); + $sandbox->terminating(function () use (&$ran) { + $ran['b'][] = 'b'; + }); + $sandbox->terminate(); + $bTerminated->push(true); + $done->push(true); + }); + + $done->pop(); + $done->pop(); + }); + + $this->assertSame(['a' => [], 'b' => ['b']], $ran); + $this->assertSame([], $this->baseTerminatingCallbacks($base)); + } + + public function test_an_after_response_job_runs_once_and_not_again_in_later_requests(): void + { + [$base, $sandbox] = $this->applications(); + $base->singleton(Dispatcher::class, fn ($app) => new Dispatcher($app)); + $base->alias(Dispatcher::class, DispatcherContract::class); + $base->make(Dispatcher::class); + $job = new CountingJob; + + Context::set('octane.request_scope', $first = new RequestScope($base, Request::create('/js-error'))); + $sandbox->make(DispatcherContract::class)->dispatchAfterResponse($job); + $sandbox->terminate(); + $first->clear(); + + foreach (['/later-1', '/later-2'] as $uri) { + Context::set('octane.request_scope', $later = new RequestScope($base, Request::create($uri))); + $sandbox->terminate(); + $later->clear(); + } + + $this->assertSame(1, $job->runs); + $this->assertSame([], $this->baseTerminatingCallbacks($base)); + } + + public function test_outside_a_request_terminating_registers_on_the_base_application(): void + { + [$base, $sandbox] = $this->applications(); + + $sandbox->terminating(fn () => null); + + $this->assertCount(1, $this->baseTerminatingCallbacks($base)); + } + + /** + * @return array{0: \Illuminate\Foundation\Application, 1: \Laravel\Octane\Swoole\Coroutine\CoroutineApplication} + */ + private function applications(): array + { + $base = new Application(__DIR__); + + return [$base, new CoroutineApplication($base)]; + } + + private function baseTerminatingCallbacks(Application $base): array + { + return (new ReflectionProperty($base, 'terminatingCallbacks'))->getValue($base); + } +} + +class CountingJob +{ + public int $runs = 0; + + public function handle(): void + { + $this->runs++; + } +} diff --git a/tests/Unit/RedisConnectionPoolTest.php b/tests/Unit/RedisConnectionPoolTest.php new file mode 100644 index 0000000..ebfc973 --- /dev/null +++ b/tests/Unit/RedisConnectionPoolTest.php @@ -0,0 +1,367 @@ +markTestSkipped('phpredis and Swoole are required.'); + } + + $socket = @fsockopen('127.0.0.1', 6379, $errno, $errstr, 0.5); + + if ($socket === false) { + $this->markTestSkipped('A Redis server on 127.0.0.1:6379 is required.'); + } + + fclose($socket); + + CoroutineRedisManager::flushPool(); + Context::clear(); + } + + protected function tearDown(): void + { + CoroutineRedisManager::flushPool(); + Context::clear(); + + parent::tearDown(); + } + + public function test_sequential_requests_reuse_one_connection(): void + { + $manager = $this->manager(); + + $first = $this->request($manager, fn (Connection $redis) => $redis->set('pool:k', 'v') ? $this->clientId($redis) : null); + $second = $this->request($manager, fn (Connection $redis) => [$this->clientId($redis), $redis->get('pool:k')]); + + $this->assertSame($first, $second[0]); + $this->assertSame('v', $second[1]); + } + + public function test_concurrent_requests_never_share_a_connection(): void + { + $manager = $this->manager(); + $ids = []; + + Coroutine\run(function () use ($manager, &$ids) { + $bothHolding = new Channel(2); + $release = new Channel(2); + $done = new Channel(2); + + for ($i = 0; $i < 2; $i++) { + Coroutine::create(function () use ($manager, $bothHolding, $release, $done) { + $id = $this->clientId($manager->connection()); + $bothHolding->push(true); + $release->pop(); + $manager->releaseConnections(); + $done->push($id); + }); + } + + $bothHolding->pop(); + $bothHolding->pop(); + $release->push(true); + $release->push(true); + $ids = [$done->pop(), $done->pop()]; + }); + + $this->assertNotSame($ids[0], $ids[1]); + } + + #[DataProvider('dirtyConnections')] + public function test_a_connection_that_could_carry_state_is_closed_not_reused(Closure $dirty): void + { + $manager = $this->manager(); + + $first = $this->request($manager, function (Connection $redis) use ($dirty) { + $id = $this->clientId($redis); + + try { + $dirty($redis); + } catch (RedisException) { + // A failed command is one of the cases under test. + } + + return $id; + }); + + $second = $this->request($manager, fn (Connection $redis) => [$this->clientId($redis), $redis->get('pool:clean')]); + + $this->assertNotSame($first, $second[0]); + $this->assertFalse($this->clientIsOpen($first)); + $this->assertNull($second[1]); + } + + public static function dirtyConnections(): array + { + return [ + 'MULTI left open' => [fn (Connection $redis) => $redis->multi()->set('pool:clean', 'leaked')], + 'pipeline left open' => [fn (Connection $redis) => $redis->client()->pipeline()->set('pool:clean', 'leaked')], + 'SELECT of another database' => [fn (Connection $redis) => $redis->select(14)], + 'a changed key prefix' => [fn (Connection $redis) => $redis->client()->setOption(Redis::OPT_PREFIX, 'other:')], + 'WATCH' => [fn (Connection $redis) => $redis->watch('pool:clean')], + 'raw SELECT' => [fn (Connection $redis) => $redis->executeRaw(['SELECT', '14'])], + 'raw MULTI' => [fn (Connection $redis) => $redis->executeRaw(['MULTI'])], + ]; + } + + public function test_a_command_cut_off_before_its_reply_is_not_reused(): void + { + $manager = $this->manager([], ['read_timeout' => 0.2]); + + $first = $this->request($manager, function (Connection $redis) { + $id = $this->clientId($redis); + + try { + $redis->blpop(['pool:empty'], 2); + } catch (RedisException) { + // The read timed out with the reply still pending. + } + + return $id; + }); + + $second = $this->request($manager, fn (Connection $redis) => [$this->clientId($redis), $redis->set('pool:after-timeout', '1')]); + + $this->assertNotSame($first, $second[0]); + $this->assertTrue($second[1]); + } + + public function test_a_connection_kept_after_its_request_cannot_reach_the_pooled_socket(): void + { + $manager = $this->manager(); + + [$kept, $id] = $this->request($manager, fn (Connection $redis) => [$redis, $this->clientId($redis)]); + $next = $this->request($manager, fn (Connection $redis) => [$this->clientId($redis), $redis->get('pool:k2')]); + + $this->assertSame($id, $next[0]); + + $this->expectException(LogicException::class); + $kept->get('pool:k2'); + } + + public function test_idle_sockets_below_the_top_of_the_stack_expire(): void + { + $manager = $this->manager(['max_idle_time' => 0.3]); + $ids = $this->twoConcurrentRequests($manager); + + // Refresh only the socket on top of the stack. + usleep(200_000); + $top = $this->request($manager, fn (Connection $redis) => $this->clientId($redis)); + $bottom = $top === $ids[0] ? $ids[1] : $ids[0]; + + // The bottom socket is now past max_idle_time; the next return prunes it. + usleep(150_000); + $this->request($manager, fn (Connection $redis) => $this->clientId($redis)); + + $this->assertFalse($this->clientIsOpen($bottom)); + $this->assertTrue($this->clientIsOpen($top)); + } + + public function test_idle_connections_above_the_cap_are_closed(): void + { + $manager = $this->manager(['max_idle' => 1]); + $ids = []; + + Coroutine\run(function () use ($manager, &$ids) { + $bothHolding = new Channel(2); + $release = new Channel(2); + $done = new Channel(2); + + for ($i = 0; $i < 2; $i++) { + Coroutine::create(function () use ($manager, $bothHolding, $release, $done) { + $id = $this->clientId($manager->connection()); + $bothHolding->push(true); + $release->pop(); + $manager->releaseConnections(); + $done->push($id); + }); + } + + $bothHolding->pop(); + $bothHolding->pop(); + $release->push(true); + $release->push(true); + $ids = [$done->pop(), $done->pop()]; + }); + + $open = array_filter($ids, fn (int $id) => $this->clientIsOpen($id)); + + $this->assertCount(1, $open); + } + + public function test_a_connection_idle_too_long_is_replaced(): void + { + $manager = $this->manager(['max_idle_time' => 0.05]); + + $first = $this->request($manager, fn (Connection $redis) => $this->clientId($redis)); + usleep(100_000); + $second = $this->request($manager, fn (Connection $redis) => $this->clientId($redis)); + + $this->assertNotSame($first, $second); + $this->assertFalse($this->clientIsOpen($first)); + } + + public function test_a_connection_the_server_dropped_is_replaced_before_use(): void + { + $manager = $this->manager(['ping_after_idle' => 0]); + + $first = $this->request($manager, fn (Connection $redis) => $this->clientId($redis)); + $admin = $this->admin(); + $admin->rawCommand('CLIENT', 'KILL', 'ID', (string) $first); + $admin->close(); + + // phpredis reconnects (with AUTH and SELECT) when it finds the socket closed; the + // ping guards against half-open sockets, where no close ever arrives. + $second = $this->request($manager, fn (Connection $redis) => [$this->clientId($redis), $redis->set('pool:after-kill', '1')]); + + $this->assertNotSame($first, $second[0]); + $this->assertTrue($second[1]); + } + + public function test_with_the_pool_disabled_every_request_closes_its_connection(): void + { + $manager = $this->manager(['enabled' => false]); + + $first = $this->request($manager, fn (Connection $redis) => [$this->clientId($redis), $redis]); + $second = $this->request($manager, fn (Connection $redis) => $this->clientId($redis)); + + $this->assertNotInstanceOf(PooledPhpRedisConnection::class, $first[1]); + $this->assertNotSame($first[0], $second); + $this->assertFalse($this->clientIsOpen($first[0])); + } + + public function test_outside_a_coroutine_the_framework_connection_is_used(): void + { + $manager = $this->manager(); + + $connection = $manager->connection(); + + $this->assertSame($connection, $manager->connection()); + $this->assertNotInstanceOf(PooledPhpRedisConnection::class, $connection); + } + + /** + * Two requests that hold a connection at the same time, then release it. + * + * @return array{0: int, 1: int} + */ + private function twoConcurrentRequests(CoroutineRedisManager $manager): array + { + $ids = []; + + Coroutine\run(function () use ($manager, &$ids) { + $bothHolding = new Channel(2); + $release = new Channel(2); + $done = new Channel(2); + + for ($i = 0; $i < 2; $i++) { + Coroutine::create(function () use ($manager, $bothHolding, $release, $done) { + $id = $this->clientId($manager->connection()); + $bothHolding->push(true); + $release->pop(); + $manager->releaseConnections(); + $done->push($id); + }); + } + + $bothHolding->pop(); + $bothHolding->pop(); + $release->push(true); + $release->push(true); + $ids = [$done->pop(), $done->pop()]; + }); + + return $ids; + } + + /** + * Run one "request": a coroutine that uses the default connection and releases it. + */ + private function request(CoroutineRedisManager $manager, Closure $callback) + { + $result = null; + + Coroutine\run(function () use ($manager, $callback, &$result) { + $result = $callback($manager->connection()); + $manager->releaseConnections(); + }); + + return $result; + } + + private function manager(array $pool = [], array $connection = []): CoroutineRedisManager + { + $redis = [ + 'default' => array_merge(['host' => '127.0.0.1', 'port' => 6379, 'database' => 15], $connection), + ]; + + $app = new Application(__DIR__); + $app->instance('config', new ConfigRepository([ + 'database' => ['redis' => $redis], + 'octane' => ['swoole' => ['redis_pool' => array_merge(['enabled' => true], $pool)]], + ])); + + CoroutineRedisManager::flushPool(); + + return new CoroutineRedisManager($app, 'phpredis', $redis); + } + + private function clientId(Connection $redis): int + { + return (int) $redis->client()->rawCommand('CLIENT', 'ID'); + } + + private function clientIsOpen(int $id): bool + { + // The server drops a closed client asynchronously; give it a moment. + for ($attempt = 0; $attempt < 20; $attempt++) { + $admin = $this->admin(); + $open = str_contains((string) $admin->rawCommand('CLIENT', 'LIST'), 'id='.$id.' '); + $admin->close(); + + if (! $open) { + return false; + } + + usleep(25_000); + } + + return true; + } + + private function admin(): Redis + { + $redis = new Redis; + $redis->connect('127.0.0.1', 6379); + + return $redis; + } +}