From 7850eed848da3beab37eb87ef0974dc751ab2270 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 14:00:03 +0200 Subject: [PATCH 01/16] Update php & Xdebug versions in docker images --- .docker/php/7.1/Dockerfile | 14 ++++++++------ .docker/php/7.2/Dockerfile | 14 ++++++++------ .docker/php/7.3/Dockerfile | 14 ++++++++------ 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/.docker/php/7.1/Dockerfile b/.docker/php/7.1/Dockerfile index ebc0cfb..0d0dc12 100644 --- a/.docker/php/7.1/Dockerfile +++ b/.docker/php/7.1/Dockerfile @@ -1,10 +1,12 @@ FROM php:7.1-fpm-alpine +ENV XDEBUG_VERSION=2.7.2 ENV PHP_CONF_DIR=/usr/local/etc -RUN apk update && apk upgrade && apk add --no-cache $PHPIZE_DEPS \ - && pecl install xdebug-2.7.1 \ +RUN apk update && apk upgrade && apk add --no-cache ${PHPIZE_DEPS} \ + && pecl install xdebug-${XDEBUG_VERSION} \ && docker-php-ext-enable xdebug \ - && apk del $PHPIZE_DEPS -COPY network-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/network-socket.pool.conf -COPY restricted-unix-domain-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/restricted-unix-domain-socket.pool.conf -COPY unix-domain-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/unix-domain-socket.pool.conf + && apk del ${PHPIZE_DEPS} \ + && rm -rf /var/cache/apk/* +COPY network-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/network-socket.pool.conf +COPY restricted-unix-domain-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/restricted-unix-domain-socket.pool.conf +COPY unix-domain-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/unix-domain-socket.pool.conf WORKDIR /repo \ No newline at end of file diff --git a/.docker/php/7.2/Dockerfile b/.docker/php/7.2/Dockerfile index 623f0d8..44fc663 100644 --- a/.docker/php/7.2/Dockerfile +++ b/.docker/php/7.2/Dockerfile @@ -1,10 +1,12 @@ FROM php:7.2-fpm-alpine +ENV XDEBUG_VERSION=2.7.2 ENV PHP_CONF_DIR=/usr/local/etc -RUN apk update && apk upgrade && apk add --no-cache $PHPIZE_DEPS \ - && pecl install xdebug-2.7.1 \ +RUN apk update && apk upgrade && apk add --no-cache ${PHPIZE_DEPS} \ + && pecl install xdebug-${XDEBUG_VERSION} \ && docker-php-ext-enable xdebug \ - && apk del $PHPIZE_DEPS -COPY network-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/network-socket.pool.conf -COPY restricted-unix-domain-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/restricted-unix-domain-socket.pool.conf -COPY unix-domain-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/unix-domain-socket.pool.conf + && apk del ${PHPIZE_DEPS} \ + && rm -rf /var/cache/apk/* +COPY network-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/network-socket.pool.conf +COPY restricted-unix-domain-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/restricted-unix-domain-socket.pool.conf +COPY unix-domain-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/unix-domain-socket.pool.conf WORKDIR /repo \ No newline at end of file diff --git a/.docker/php/7.3/Dockerfile b/.docker/php/7.3/Dockerfile index fc1b98a..fd1923a 100644 --- a/.docker/php/7.3/Dockerfile +++ b/.docker/php/7.3/Dockerfile @@ -1,10 +1,12 @@ FROM php:7.3-fpm-alpine +ENV XDEBUG_VERSION=2.7.2 ENV PHP_CONF_DIR=/usr/local/etc -RUN apk update && apk upgrade && apk add --no-cache $PHPIZE_DEPS \ - && pecl install xdebug-2.7.1 \ +RUN apk update && apk upgrade && apk add --no-cache ${PHPIZE_DEPS} \ + && pecl install xdebug-${XDEBUG_VERSION} \ && docker-php-ext-enable xdebug \ - && apk del $PHPIZE_DEPS -COPY network-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/network-socket.pool.conf -COPY restricted-unix-domain-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/restricted-unix-domain-socket.pool.conf -COPY unix-domain-socket.pool.conf $PHP_CONF_DIR/php-fpm.d/unix-domain-socket.pool.conf + && apk del ${PHPIZE_DEPS} \ + && rm -rf /var/cache/apk/* +COPY network-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/network-socket.pool.conf +COPY restricted-unix-domain-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/restricted-unix-domain-socket.pool.conf +COPY unix-domain-socket.pool.conf ${PHP_CONF_DIR}/php-fpm.d/unix-domain-socket.pool.conf WORKDIR /repo \ No newline at end of file From 552c804dd6929586ff26a2dd8ef5775508ff6265 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 14:16:32 +0200 Subject: [PATCH 02/16] Add PHPUnit result cache to ignored VCS files --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index b65a748..9c6ea52 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ /.idea/ /vendor/ /build/logs/ -/.phpunit.result.cache +/build/.phpunit.result.cache /composer.lock \ No newline at end of file From 91cc70db48d5a2bf1492a69a06ffdc70c9fa7865 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 14:17:12 +0200 Subject: [PATCH 03/16] Add failing tests to reproduce the issue, #40 --- tests/Integration/AsyncRequestsTest.php | 143 ++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 tests/Integration/AsyncRequestsTest.php diff --git a/tests/Integration/AsyncRequestsTest.php b/tests/Integration/AsyncRequestsTest.php new file mode 100644 index 0000000..3e8ddf2 --- /dev/null +++ b/tests/Integration/AsyncRequestsTest.php @@ -0,0 +1,143 @@ +getMaxChildrenSettingFromNetworkSocket(); + $limit = $maxChildren + 5; + + $this->assertTrue( $limit > 5 ); + + $client = $this->getClientWithNetworkSocket(); + $results = []; + $expectedResults = range( 0, $limit - 1 ); + + $request = new PostRequest( __DIR__ . '/Workers/worker.php', '' ); + $request->addResponseCallbacks( + static function ( ProvidesResponseData $response ) use ( &$results ) + { + $results[] = $response->getBody(); + } + ); + + for ( $i = 0; $i < $limit; $i++ ) + { + $request->setContent( http_build_query( ['test-key' => $i] ) ); + + $client->sendAsyncRequest( $request ); + } + + $client->waitForResponses(); + + $this->assertEquals( $expectedResults, $results ); + } + + private function getMaxChildrenSettingFromNetworkSocket() : int + { + $iniSettings = parse_ini_file( + __DIR__ . '/../../.docker/php/network-socket.pool.conf', + true + ); + + return (int)$iniSettings['network']['pm.max_children']; + } + + private function getClientWithNetworkSocket() : Client + { + $networkSocket = new NetworkSocket( + $this->getNetworkSocketHost(), + $this->getNetworkSocketPort() + ); + + return new Client( $networkSocket ); + } + + /** + * @throws ConnectException + * @throws ExpectationFailedException + * @throws InvalidArgumentException + * @throws ReadFailedException + * @throws Throwable + * @throws TimedoutException + * @throws WriteFailedException + */ + public function testAsyncRequestsWillRespondIfRequestsExceedPhpFpmMaxChildrenSettingOnUnixDomainSocket() : void + { + $maxChildren = $this->getMaxChildrenSettingFromUnixDomainSocket(); + $limit = $maxChildren + 5; + + $this->assertTrue( $limit > 5 ); + + $client = $this->getClientWithUnixDomainSocket(); + $results = []; + $expectedResults = range( 0, $limit - 1 ); + + $request = new PostRequest( __DIR__ . '/Workers/worker.php', '' ); + $request->addResponseCallbacks( + static function ( ProvidesResponseData $response ) use ( &$results ) + { + $results[] = $response->getBody(); + } + ); + + for ( $i = 0; $i < $limit; $i++ ) + { + $request->setContent( http_build_query( ['test-key' => $i] ) ); + + $client->sendAsyncRequest( $request ); + } + + $client->waitForResponses(); + + $this->assertEquals( $expectedResults, $results ); + } + + private function getMaxChildrenSettingFromUnixDomainSocket() : int + { + $iniSettings = parse_ini_file( + __DIR__ . '/../../.docker/php/unix-domain-socket.pool.conf', + true + ); + + return (int)$iniSettings['uds']['pm.max_children']; + } + + private function getClientWithUnixDomainSocket() : Client + { + $unixDomainSocket = new UnixDomainSocket( $this->getUnixDomainSocket() ); + + return new Client( $unixDomainSocket ); + } +} From af127dc862cb6a6b0a5f30d0f61c1089406648d8 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 14:22:19 +0200 Subject: [PATCH 04/16] Fix comparison of expected results in tests, #40 --- tests/Integration/AsyncRequestsTest.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/Integration/AsyncRequestsTest.php b/tests/Integration/AsyncRequestsTest.php index 3e8ddf2..2b504b7 100644 --- a/tests/Integration/AsyncRequestsTest.php +++ b/tests/Integration/AsyncRequestsTest.php @@ -19,6 +19,7 @@ use function http_build_query; use function parse_ini_file; use function range; +use function sort; final class AsyncRequestsTest extends TestCase { @@ -48,7 +49,7 @@ public function testAsyncRequestsWillRespondIfRequestsExceedPhpFpmMaxChildrenSet $request->addResponseCallbacks( static function ( ProvidesResponseData $response ) use ( &$results ) { - $results[] = $response->getBody(); + $results[] = (int)$response->getBody(); } ); @@ -61,6 +62,8 @@ static function ( ProvidesResponseData $response ) use ( &$results ) $client->waitForResponses(); + sort( $results ); + $this->assertEquals( $expectedResults, $results ); } @@ -108,7 +111,7 @@ public function testAsyncRequestsWillRespondIfRequestsExceedPhpFpmMaxChildrenSet $request->addResponseCallbacks( static function ( ProvidesResponseData $response ) use ( &$results ) { - $results[] = $response->getBody(); + $results[] = (int)$response->getBody(); } ); @@ -121,6 +124,8 @@ static function ( ProvidesResponseData $response ) use ( &$results ) $client->waitForResponses(); + sort( $results ); + $this->assertEquals( $expectedResults, $results ); } From b85e3bd55b49e9e9d992add7e6620a42f16bde10 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 14:32:33 +0200 Subject: [PATCH 05/16] Add failing tests to reproduce the issue when reading responses, #40 * Those tests cover the issue when not using response callbacks --- tests/Integration/AsyncRequestsTest.php | 99 ++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 4 deletions(-) diff --git a/tests/Integration/AsyncRequestsTest.php b/tests/Integration/AsyncRequestsTest.php index 2b504b7..053340d 100644 --- a/tests/Integration/AsyncRequestsTest.php +++ b/tests/Integration/AsyncRequestsTest.php @@ -34,7 +34,8 @@ final class AsyncRequestsTest extends TestCase * @throws TimedoutException * @throws WriteFailedException */ - public function testAsyncRequestsWillRespondIfRequestsExceedPhpFpmMaxChildrenSettingOnNetworkSocket() : void + public function testAsyncRequestsWillRespondToCallbackIfRequestsExceedPhpFpmMaxChildrenSettingOnNetworkSocket( + ) : void { $maxChildren = $this->getMaxChildrenSettingFromNetworkSocket(); $limit = $maxChildren + 5; @@ -64,7 +65,7 @@ static function ( ProvidesResponseData $response ) use ( &$results ) sort( $results ); - $this->assertEquals( $expectedResults, $results ); + $this->assertSame( $expectedResults, $results ); } private function getMaxChildrenSettingFromNetworkSocket() : int @@ -96,7 +97,8 @@ private function getClientWithNetworkSocket() : Client * @throws TimedoutException * @throws WriteFailedException */ - public function testAsyncRequestsWillRespondIfRequestsExceedPhpFpmMaxChildrenSettingOnUnixDomainSocket() : void + public function testAsyncRequestsWillRespondToCallbackIfRequestsExceedPhpFpmMaxChildrenSettingOnUnixDomainSocket( + ) : void { $maxChildren = $this->getMaxChildrenSettingFromUnixDomainSocket(); $limit = $maxChildren + 5; @@ -126,7 +128,7 @@ static function ( ProvidesResponseData $response ) use ( &$results ) sort( $results ); - $this->assertEquals( $expectedResults, $results ); + $this->assertSame( $expectedResults, $results ); } private function getMaxChildrenSettingFromUnixDomainSocket() : int @@ -145,4 +147,93 @@ private function getClientWithUnixDomainSocket() : Client return new Client( $unixDomainSocket ); } + + /** + * @throws ConnectException + * @throws ExpectationFailedException + * @throws InvalidArgumentException + * @throws ReadFailedException + * @throws TimedoutException + * @throws WriteFailedException + */ + public function testCanReadResponsesOfAsyncRequestsIfRequestsExceedPhpFpmMaxChildrenSettingOnNetworkSocket() : void + { + $maxChildren = $this->getMaxChildrenSettingFromNetworkSocket(); + $limit = $maxChildren + 5; + + $this->assertTrue( $limit > 5 ); + + $client = $this->getClientWithNetworkSocket(); + $results = []; + $expectedResults = range( 0, $limit - 1 ); + + $request = new PostRequest( __DIR__ . '/Workers/worker.php', '' ); + + for ( $i = 0; $i < $limit; $i++ ) + { + $request->setContent( http_build_query( ['test-key' => $i] ) ); + + $client->sendAsyncRequest( $request ); + } + + while ( $client->hasUnhandledResponses() ) + { + foreach ( $client->readReadyResponses() as $response ) + { + $results[] = (int)$response->getBody(); + } + } + + sort( $results ); + + $this->assertSame( $expectedResults, $results ); + } + + /** + * @throws ConnectException + * @throws ExpectationFailedException + * @throws InvalidArgumentException + * @throws ReadFailedException + * @throws TimedoutException + * @throws WriteFailedException + */ + public function testCanReadResponsesOfAsyncRequestsIfRequestsExceedPhpFpmMaxChildrenSettingOnUnixDomainSocket( + ) : void + { + $maxChildren = $this->getMaxChildrenSettingFromUnixDomainSocket(); + $limit = $maxChildren + 5; + + $this->assertTrue( $limit > 5 ); + + $client = $this->getClientWithUnixDomainSocket(); + $results = []; + $expectedResults = range( 0, $limit - 1 ); + + $request = new PostRequest( __DIR__ . '/Workers/worker.php', '' ); + $request->addResponseCallbacks( + static function ( ProvidesResponseData $response ) use ( &$results ) + { + $results[] = (int)$response->getBody(); + } + ); + + for ( $i = 0; $i < $limit; $i++ ) + { + $request->setContent( http_build_query( ['test-key' => $i] ) ); + + $client->sendAsyncRequest( $request ); + } + + while ( $client->hasUnhandledResponses() ) + { + foreach ( $client->readReadyResponses() as $response ) + { + $results[] = (int)$response->getBody(); + } + } + + sort( $results ); + + $this->assertSame( $expectedResults, $results ); + } } From 8190f11e63cdad39db8da0f13623e6ee9c791408 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 14:56:33 +0200 Subject: [PATCH 06/16] Remove socket from collection to prevent halt on further processing, #40 --- src/Client.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Client.php b/src/Client.php index 132e47e..e53baa5 100644 --- a/src/Client.php +++ b/src/Client.php @@ -193,10 +193,12 @@ private function fetchResponseAndNotifyCallback( Socket $socket, ?int $timeoutMs } catch ( Throwable $e ) { - $this->sockets->remove( $socket->getId() ); - $socket->notifyFailureCallbacks( $e ); } + finally + { + $this->sockets->remove( $socket->getId() ); + } } /** @@ -264,6 +266,10 @@ public function readResponses( ?int $timeoutMs = null, int ...$requestIds ) : Ge yield $this->sockets->getById( $requestId )->fetchResponse( $timeoutMs ); } catch ( Throwable $e ) + { + # Skip unknown request ids + } + finally { $this->sockets->remove( $requestId ); } From 9c2a5c0295ec1f8ecc5bdc7f01edc52e5e50024a Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 15:03:58 +0200 Subject: [PATCH 07/16] Update changelog, #40 --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47a746c..6492ae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a CHANGELOG](http://keepachangelog.com). +## [2.7.2] - YYYY-MM-DD + +### Fixed + +* Remove/close sockets after fetching their responses triggered async requests in order to prevent halt on further + request processing, if the number of requests exceeds php-fpm's `pm.max_children` setting. - [#40] + ## [2.7.1] - 2019-04-29 ### Fixed @@ -193,6 +200,7 @@ Based on [Pierrick Charron](https://github.com/adoy)'s [PHP-FastCGI-Client](http * Getters/Setters for connect timeout, read/write timeout, keep alive, socket persistence from `Client` (now part of the socket connection) * Method `Client->getValues()` +[2.7.2]: https://github.com/hollodotme/fast-cgi-client/compare/v2.7.1...v2.7.2 [2.7.1]: https://github.com/hollodotme/fast-cgi-client/compare/v2.7.0...v2.7.1 [2.7.0]: https://github.com/hollodotme/fast-cgi-client/compare/v2.6.0...v2.7.0 [2.6.0]: https://github.com/hollodotme/fast-cgi-client/compare/v2.5.0...v2.6.0 @@ -221,3 +229,4 @@ Based on [Pierrick Charron](https://github.com/adoy)'s [PHP-FastCGI-Client](http [#27]: https://github.com/hollodotme/fast-cgi-client/issues/27 [#33]: https://github.com/hollodotme/fast-cgi-client/pull/33 [#37]: https://github.com/hollodotme/fast-cgi-client/issue/37 +[#40]: https://github.com/hollodotme/fast-cgi-client/issue/40 From e036cb14607fc04fb9ed8571575a4a3461b4c90c Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 18:08:34 +0200 Subject: [PATCH 08/16] Remove obsolete method Client#getSocketsHavingResponse() * This method was used only once in a loop that could be substituted by Client#handleReadyResponses() --- src/Client.php | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/Client.php b/src/Client.php index e53baa5..afe83d7 100644 --- a/src/Client.php +++ b/src/Client.php @@ -172,10 +172,7 @@ public function waitForResponses( ?int $timeoutMs = null ) : void while ( $this->hasUnhandledResponses() ) { - foreach ( $this->getSocketsHavingResponse() as $socket ) - { - $this->fetchResponseAndNotifyCallback( $socket, $timeoutMs ); - } + $this->handleReadyResponses( $timeoutMs ); } } @@ -209,18 +206,6 @@ public function hasUnhandledResponses() : bool return $this->sockets->hasBusySockets(); } - /** - * @return Generator|Socket[] - * @throws ReadFailedException - */ - private function getSocketsHavingResponse() : Generator - { - foreach ( $this->getRequestIdsHavingResponse() as $requestId ) - { - yield $this->sockets->getById( $requestId ); - } - } - /** * @param int $requestId * @@ -246,7 +231,7 @@ public function getRequestIdsHavingResponse() : array $reads = $this->sockets->collectResources(); $writes = $excepts = null; - stream_select( $reads, $writes, $excepts, 0, Socket::STREAM_SELECT_USEC ); + @stream_select( $reads, $writes, $excepts, 0, Socket::STREAM_SELECT_USEC ); return $this->sockets->getSocketIdsByResources( $reads ); } From 819aa6e0115c4896f6a28ef67b48e9746ff09c44 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 18:22:22 +0200 Subject: [PATCH 09/16] Add read/write shutdown before closing/removing a socket connection, #41 --- src/Sockets/Socket.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Sockets/Socket.php b/src/Sockets/Socket.php index 3081c64..6e0318b 100644 --- a/src/Sockets/Socket.php +++ b/src/Sockets/Socket.php @@ -52,8 +52,10 @@ use function stream_select; use function stream_set_timeout; use function stream_socket_client; +use function stream_socket_shutdown; use function strlen; use function substr; +use const STREAM_SHUT_RDWR; final class Socket { @@ -541,6 +543,7 @@ private function disconnect() : void { if ( is_resource( $this->resource ) ) { + @stream_socket_shutdown( $this->resource, STREAM_SHUT_RDWR ); fclose( $this->resource ); } } From 59c2a155db4c8655df565f124198d450f1e557e9 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 19:25:00 +0200 Subject: [PATCH 10/16] Update package keywords --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index f34e504..41716ee 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,8 @@ "keywords": [ "fastcgi", "php-fpm", - "socket" + "socket", + "async" ], "minimum-stability": "dev", "prefer-stable": true, From e37000d5cadc5f586b6b2076813dbc509639610f Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Thu, 30 May 2019 19:41:24 +0200 Subject: [PATCH 11/16] Add tests to verify behavior if php-fpm process gets interrupted, #41 * Interrupt single php-fpm process with signals SIGHUP(1), SIGINT(2), SIGKILL(9) & SIGTERM(15) * Expect one of three requests to fail and the other two to respond successfully --- tests/Integration/SignaledWorkersTest.php | 199 ++++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 tests/Integration/SignaledWorkersTest.php diff --git a/tests/Integration/SignaledWorkersTest.php b/tests/Integration/SignaledWorkersTest.php new file mode 100644 index 0000000..073a865 --- /dev/null +++ b/tests/Integration/SignaledWorkersTest.php @@ -0,0 +1,199 @@ +getClientWithNetworkSocket(); + $request = new PostRequest( __DIR__ . '/Workers/worker.php', '' ); + $success = []; + $failures = []; + + $request->addResponseCallbacks( + static function ( ProvidesResponseData $response ) use ( &$success ) + { + $success[] = (int)$response->getBody(); + } + ); + + $request->addFailureCallbacks( + static function ( Throwable $e ) use ( &$failures ) + { + $failures[] = $e; + } + ); + + for ( $i = 0; $i < 3; $i++ ) + { + $request->setContent( http_build_query( ['test-key' => $i] ) ); + + $client->sendAsyncRequest( $request ); + } + + $pids = $this->getPoolWorkerPIDs( 'pool network' ); + + $this->killPoolWorker( (int)$pids[0], $signal ); + + $client->waitForResponses(); + + $this->assertCount( 2, $success ); + $this->assertCount( 1, $failures ); + $this->assertContainsOnlyInstancesOf( ReadFailedException::class, $failures ); + + sleep( 1 ); + } + + public function signalProvider() : array + { + return [ + [ + # SIGHUP + 'signal' => 1, + ], + [ + # SIGINT + 'signal' => 2, + ], + [ + # SIGKILL + 'signal' => 9, + ], + [ + # SIGTERM + 'signal' => 15, + ], + ]; + } + + private function getClientWithNetworkSocket() : Client + { + $networkSocket = new NetworkSocket( + $this->getNetworkSocketHost(), + $this->getNetworkSocketPort() + ); + + return new Client( $networkSocket ); + } + + private function getPoolWorkerPIDs( string $poolName ) : array + { + $command = sprintf( + 'ps -o pid,args | grep %s | grep -v "grep"', + escapeshellarg( $poolName ) + ); + $list = shell_exec( $command ); + + return array_map( + static function ( string $item ) + { + preg_match( '#^(\d+)\s.+$#', trim( $item ), $matches ); + + return (int)$matches[1]; + }, + explode( "\n", $list ) + ); + } + + private function killPoolWorker( int $PID, int $signal ) : void + { + $command = sprintf( 'kill -%d %d', $signal, $PID ); + exec( $command ); + } + + /** + * @param int $signal + * + * @throws ConnectException + * @throws ExpectationFailedException + * @throws InvalidArgumentException + * @throws ReadFailedException + * @throws Throwable + * @throws TimedoutException + * @throws WriteFailedException + * @dataProvider signalProvider + */ + public function testFailureCallbackGetsCalledIfOneProcessGetsInterruptedOnUnixDomainSocket( int $signal ) : void + { + $client = $this->getClientWithUnixDomainSocket(); + $request = new PostRequest( __DIR__ . '/Workers/worker.php', '' ); + $success = []; + $failures = []; + + $request->addResponseCallbacks( + static function ( ProvidesResponseData $response ) use ( &$success ) + { + $success[] = (int)$response->getBody(); + } + ); + + $request->addFailureCallbacks( + static function ( Throwable $e ) use ( &$failures ) + { + $failures[] = $e; + } + ); + + for ( $i = 0; $i < 3; $i++ ) + { + $request->setContent( http_build_query( ['test-key' => $i] ) ); + + $client->sendAsyncRequest( $request ); + } + + $pids = $this->getPoolWorkerPIDs( 'pool uds' ); + + $this->killPoolWorker( (int)$pids[0], $signal ); + + $client->waitForResponses(); + + $this->assertCount( 2, $success ); + $this->assertCount( 1, $failures ); + $this->assertContainsOnlyInstancesOf( ReadFailedException::class, $failures ); + + sleep( 1 ); + } + + private function getClientWithUnixDomainSocket() : Client + { + $unixDomainSocket = new UnixDomainSocket( $this->getUnixDomainSocket() ); + + return new Client( $unixDomainSocket ); + } +} From c44f1a0c89edc1537de1abffbfb68f21c9887a0c Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Fri, 31 May 2019 00:06:49 +0200 Subject: [PATCH 12/16] Add tests to verify behavior if php-fpm process gets interrupted, #41 * Interrupt multiple php-fpm process with signals SIGHUP(1), SIGINT(2), SIGKILL(9) & SIGTERM(15) * Expect all three requests to fail --- tests/Integration/SignaledWorkersTest.php | 121 +++++++++++++++++++++- 1 file changed, 120 insertions(+), 1 deletion(-) diff --git a/tests/Integration/SignaledWorkersTest.php b/tests/Integration/SignaledWorkersTest.php index 073a865..1439912 100644 --- a/tests/Integration/SignaledWorkersTest.php +++ b/tests/Integration/SignaledWorkersTest.php @@ -17,10 +17,13 @@ use SebastianBergmann\RecursionContext\InvalidArgumentException; use Throwable; use function escapeshellarg; +use function exec; use function http_build_query; +use function implode; use function preg_match; use function shell_exec; use function sleep; +use function sprintf; final class SignaledWorkersTest extends TestCase { @@ -127,7 +130,7 @@ static function ( string $item ) return (int)$matches[1]; }, - explode( "\n", $list ) + explode( "\n", trim( $list ) ) ); } @@ -196,4 +199,120 @@ private function getClientWithUnixDomainSocket() : Client return new Client( $unixDomainSocket ); } + + /** + * @param int $signal + * + * @throws ConnectException + * @throws ExpectationFailedException + * @throws \InvalidArgumentException + * @throws ReadFailedException + * @throws Throwable + * @throws TimedoutException + * @throws WriteFailedException + * + * @dataProvider signalProvider + */ + public function testFailureCallbackGetsCalledIfAllProcessesGetInterruptedOnNetworkSocket( int $signal ) : void + { + $client = $this->getClientWithNetworkSocket(); + $request = new PostRequest( __DIR__ . '/Workers/sleepWorker.php', '' ); + $success = []; + $failures = []; + + $request->addResponseCallbacks( + static function ( ProvidesResponseData $response ) use ( &$success ) + { + $success[] = (int)$response->getBody(); + } + ); + + $request->addFailureCallbacks( + static function ( Throwable $e ) use ( &$failures ) + { + $failures[] = $e; + } + ); + + for ( $i = 0; $i < 3; $i++ ) + { + $request->setContent( http_build_query( ['test-key' => $i, 'sleep' => 1] ) ); + + $client->sendAsyncRequest( $request ); + } + + $this->killPhpFpmChildProcesses( 'pool network', $signal ); + + $client->waitForResponses(); + + $this->assertCount( 0, $success ); + $this->assertCount( 3, $failures ); + $this->assertContainsOnlyInstancesOf( ReadFailedException::class, $failures ); + + sleep( 1 ); + } + + private function killPhpFpmChildProcesses( string $poolName, int $signal ) : void + { + $PIDs = $this->getPoolWorkerPIDs( $poolName ); + $this->killPoolWorkers( $PIDs, $signal ); + } + + private function killPoolWorkers( array $PIDs, int $signal ) : void + { + $command = sprintf( 'kill -%d %s', $signal, implode( ' ', $PIDs ) ); + exec( $command ); + } + + /** + * @param int $signal + * + * @throws ConnectException + * @throws ExpectationFailedException + * @throws \InvalidArgumentException + * @throws ReadFailedException + * @throws Throwable + * @throws TimedoutException + * @throws WriteFailedException + * + * @dataProvider signalProvider + */ + public function testFailureCallbackGetsCalledIfAllProcessesGetInterruptedOnUnixDomainSocket( int $signal ) : void + { + $client = $this->getClientWithUnixDomainSocket(); + $request = new PostRequest( __DIR__ . '/Workers/sleepWorker.php', '' ); + $success = []; + $failures = []; + + $request->addResponseCallbacks( + static function ( ProvidesResponseData $response ) use ( &$success ) + { + $success[] = (int)$response->getBody(); + } + ); + + $request->addFailureCallbacks( + static function ( Throwable $e ) use ( &$failures ) + { + $failures[] = $e; + } + ); + + for ( $i = 0; $i < 3; $i++ ) + { + $request->setContent( http_build_query( ['test-key' => $i, 'sleep' => 1] ) ); + + $client->sendAsyncRequest( $request ); + } + + $this->killPhpFpmChildProcesses( 'pool uds', $signal ); + + $client->waitForResponses(); + + $this->assertCount( 0, $success ); + $this->assertCount( 3, $failures ); + $this->assertContainsOnlyInstancesOf( ReadFailedException::class, $failures ); + + sleep( 1 ); + } } From 19dc73f05162107c3d82e0926b2b55b51049f23c Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Fri, 31 May 2019 00:08:29 +0200 Subject: [PATCH 13/16] Handle false return value of stream_select, #41 --- src/Client.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Client.php b/src/Client.php index afe83d7..18f5ea5 100644 --- a/src/Client.php +++ b/src/Client.php @@ -231,7 +231,12 @@ public function getRequestIdsHavingResponse() : array $reads = $this->sockets->collectResources(); $writes = $excepts = null; - @stream_select( $reads, $writes, $excepts, 0, Socket::STREAM_SELECT_USEC ); + $result = @stream_select( $reads, $writes, $excepts, 0, Socket::STREAM_SELECT_USEC ); + + if ( false === $result || 0 === count( $reads ) ) + { + return []; + } return $this->sockets->getSocketIdsByResources( $reads ); } From e2d0d6e87aeef30900327d7151602e292c6b86e7 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Fri, 31 May 2019 00:08:57 +0200 Subject: [PATCH 14/16] Update output for local test script --- tests/runTestsOnAllLocalPhpVersions.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/runTestsOnAllLocalPhpVersions.sh b/tests/runTestsOnAllLocalPhpVersions.sh index decf861..722d07d 100755 --- a/tests/runTestsOnAllLocalPhpVersions.sh +++ b/tests/runTestsOnAllLocalPhpVersions.sh @@ -3,12 +3,15 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )")" cd "$(dirname "${DIR}" )" >/dev/null 2>&1 -docker-compose up -d +docker-compose up -d php71 php72 php73 echo -e "\n\033[43mRun PHPUnit\033[0m\n" +echo -e "\n\033[33mOn PHP 7.1\033[0m\n" docker-compose exec php71 vendor/bin/phpunit7.phar -c build +echo -e "\n\033[33mOn PHP 7.2\033[0m\n" docker-compose exec php72 vendor/bin/phpunit8.phar -c build +echo -e "\n\033[33mOn PHP 7.3\033[0m\n" docker-compose exec php73 vendor/bin/phpunit8.phar -c build echo -e "\n\033[43mRun phpstan\033[0m\n" -docker-compose run phpstan \ No newline at end of file +docker-compose run --rm phpstan \ No newline at end of file From b3a37ac4e3fdf9d254503b68f5e9b99bcebe8868 Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Fri, 31 May 2019 00:10:51 +0200 Subject: [PATCH 15/16] Update changelog, #41 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6492ae5..7cc25df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a CH ## [2.7.2] - YYYY-MM-DD +### Improved + +* Handling of `stream_select` returning `false` in case of a system call interrupt. - [#41] + ### Fixed * Remove/close sockets after fetching their responses triggered async requests in order to prevent halt on further @@ -230,3 +234,4 @@ Based on [Pierrick Charron](https://github.com/adoy)'s [PHP-FastCGI-Client](http [#33]: https://github.com/hollodotme/fast-cgi-client/pull/33 [#37]: https://github.com/hollodotme/fast-cgi-client/issue/37 [#40]: https://github.com/hollodotme/fast-cgi-client/issue/40 +[#41]: https://github.com/hollodotme/fast-cgi-client/issue/41 From c02a53355fe4b6edc80126fab7ba15f747e7bcff Mon Sep 17 00:00:00 2001 From: Holger Woltersdorf Date: Fri, 31 May 2019 00:42:44 +0200 Subject: [PATCH 16/16] Add release date, bumping v2.7.2 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cc25df..efee02d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a CHANGELOG](http://keepachangelog.com). -## [2.7.2] - YYYY-MM-DD +## [2.7.2] - 2019-05-31 ### Improved