From 0ae685878cc6641b83b9566c4bdb2fe83c2f218b Mon Sep 17 00:00:00 2001 From: Max Voloshin Date: Wed, 6 Nov 2013 00:13:55 +0200 Subject: [PATCH 1/2] [Process] fixed fatal errors in getOutput and getErrorOutput when process was not started --- src/Symfony/Component/Process/Process.php | 17 +++++++++++ .../Process/Tests/AbstractProcessTest.php | 30 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 33cb381397..9d7c81e880 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -296,9 +296,14 @@ class Process * * @throws RuntimeException When process timed out * @throws RuntimeException When process stopped after receiving signal + * @throws LogicException When process is not started */ public function wait($callback = null) { + if (!$this->isStarted()) { + throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__)); + } + $this->updateStatus(false); if (null !== $callback) { $this->callback = $this->buildCallback($callback); @@ -366,10 +371,16 @@ class Process * * @return string The process output * + * @throws LogicException In case the process is not started + * * @api */ public function getOutput() { + if (!$this->isStarted()) { + throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__)); + } + $this->readPipes(false, defined('PHP_WINDOWS_VERSION_BUILD') ? !$this->processInformation['running'] : true); return $this->stdout; @@ -398,10 +409,16 @@ class Process * * @return string The process error output * + * @throws LogicException In case the process is not started + * * @api */ public function getErrorOutput() { + if (!$this->isStarted()) { + throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__)); + } + $this->readPipes(false, defined('PHP_WINDOWS_VERSION_BUILD') ? !$this->processInformation['running'] : true); return $this->stderr; diff --git a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php index 5be61b03c6..f6b68d5884 100644 --- a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php +++ b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php @@ -617,6 +617,36 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase $process->signal(SIGHUP); } + /** + * @expectedException \Symfony\Component\Process\Exception\LogicException + * @expectedExceptionMessage Process must be started before calling getOutput + */ + public function testGetOutputProcessNotStarted() + { + $process = $this->getProcess('php -m'); + $process->getOutput(); + } + + /** + * @expectedException \Symfony\Component\Process\Exception\LogicException + * @expectedExceptionMessage Process must be started before calling getErrorOutput + */ + public function testGetErrorOutputProcessNotStarted() + { + $process = $this->getProcess('php -m'); + $process->getErrorOutput(); + } + + /** + * @expectedException \Symfony\Component\Process\Exception\LogicException + * @expectedExceptionMessage Process must be started before calling wait + */ + public function testWaitProcessWithoutTimeoutNotStarted() + { + $process = $this->getProcess('php -m')->setTimeout(null); + $process->wait(); + } + private function verifyPosixIsEnabled() { if (defined('PHP_WINDOWS_VERSION_BUILD')) { From 449fe019921b83609de49c9fa5ae07f857832532 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 18 Mar 2014 16:17:31 +0100 Subject: [PATCH 2/2] [Process] Trow exceptions in case a Process method is supposed to be called after termination --- src/Symfony/Component/Process/Process.php | 78 +++++++++++++++---- .../Process/Tests/AbstractProcessTest.php | 70 +++++++++++++---- 2 files changed, 119 insertions(+), 29 deletions(-) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 9d7c81e880..5c03cc2dda 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -296,13 +296,11 @@ class Process * * @throws RuntimeException When process timed out * @throws RuntimeException When process stopped after receiving signal - * @throws LogicException When process is not started + * @throws LogicException When process is not yet started */ public function wait($callback = null) { - if (!$this->isStarted()) { - throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__)); - } + $this->requireProcessIsStarted(__FUNCTION__); $this->updateStatus(false); if (null !== $callback) { @@ -377,9 +375,7 @@ class Process */ public function getOutput() { - if (!$this->isStarted()) { - throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__)); - } + $this->requireProcessIsStarted(__FUNCTION__); $this->readPipes(false, defined('PHP_WINDOWS_VERSION_BUILD') ? !$this->processInformation['running'] : true); @@ -392,10 +388,14 @@ class Process * In comparison with the getOutput method which always return the whole * output, this one returns the new output since the last call. * + * @throws LogicException In case the process is not started + * * @return string The process output since the last call */ public function getIncrementalOutput() { + $this->requireProcessIsStarted(__FUNCTION__); + $data = $this->getOutput(); $latest = substr($data, $this->incrementalOutputOffset); @@ -415,9 +415,7 @@ class Process */ public function getErrorOutput() { - if (!$this->isStarted()) { - throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__)); - } + $this->requireProcessIsStarted(__FUNCTION__); $this->readPipes(false, defined('PHP_WINDOWS_VERSION_BUILD') ? !$this->processInformation['running'] : true); @@ -431,10 +429,14 @@ class Process * whole error output, this one returns the new error output since the last * call. * + * @throws LogicException In case the process is not started + * * @return string The process error output since the last call */ public function getIncrementalErrorOutput() { + $this->requireProcessIsStarted(__FUNCTION__); + $data = $this->getErrorOutput(); $latest = substr($data, $this->incrementalErrorOutputOffset); @@ -446,7 +448,7 @@ class Process /** * Returns the exit code returned by the process. * - * @return integer The exit status code + * @return null|integer The exit status code, null if the Process is not terminated * * @throws RuntimeException In case --enable-sigchild is activated and the sigchild compatibility mode is disabled * @@ -469,14 +471,18 @@ class Process * This method relies on the Unix exit code status standardization * and might not be relevant for other operating systems. * - * @return string A string representation for the exit status code + * @return null|string A string representation for the exit status code, null if the Process is not terminated. + * + * @throws RuntimeException In case --enable-sigchild is activated and the sigchild compatibility mode is disabled * * @see http://tldp.org/LDP/abs/html/exitcodes.html * @see http://en.wikipedia.org/wiki/Unix_signal */ public function getExitCodeText() { - $exitcode = $this->getExitCode(); + if (null === $exitcode = $this->getExitCode()) { + return; + } return isset(self::$exitCodes[$exitcode]) ? self::$exitCodes[$exitcode] : 'Unknown error'; } @@ -501,11 +507,14 @@ class Process * @return Boolean * * @throws RuntimeException In case --enable-sigchild is activated + * @throws LogicException In case the process is not terminated. * * @api */ public function hasBeenSignaled() { + $this->requireProcessIsTerminated(__FUNCTION__); + if ($this->isSigchildEnabled()) { throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved.'); } @@ -523,11 +532,14 @@ class Process * @return integer * * @throws RuntimeException In case --enable-sigchild is activated + * @throws LogicException In case the process is not terminated. * * @api */ public function getTermSignal() { + $this->requireProcessIsTerminated(__FUNCTION__); + if ($this->isSigchildEnabled()) { throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved.'); } @@ -544,10 +556,14 @@ class Process * * @return Boolean * + * @throws LogicException In case the process is not terminated. + * * @api */ public function hasBeenStopped() { + $this->requireProcessIsTerminated(__FUNCTION__); + $this->updateStatus(false); return $this->processInformation['stopped']; @@ -560,10 +576,14 @@ class Process * * @return integer * + * @throws LogicException In case the process is not terminated. + * * @api */ public function getStopSignal() { + $this->requireProcessIsTerminated(__FUNCTION__); + $this->updateStatus(false); return $this->processInformation['stopsig']; @@ -945,6 +965,10 @@ class Process */ public function checkTimeout() { + if ($this->status !== self::STATUS_STARTED) { + return; + } + if (null !== $this->timeout && $this->timeout < microtime(true) - $this->starttime) { $this->stop(0); @@ -1162,4 +1186,32 @@ class Process return true; } + + /** + * Ensures the process is running or terminated, throws a LogicException if the process has a not started. + * + * @param $functionName The function name that was called. + * + * @throws LogicException If the process has not run. + */ + private function requireProcessIsStarted($functionName) + { + if (!$this->isStarted()) { + throw new LogicException(sprintf('Process must be started before calling %s.', $functionName)); + } + } + + /** + * Ensures the process is terminated, throws a LogicException if the process has a status different than `terminated`. + * + * @param $functionName The function name that was called. + * + * @throws LogicException If the process is not yet terminated. + */ + private function requireProcessIsTerminated($functionName) + { + if (!$this->isTerminated()) { + throw new LogicException(sprintf('Process must be terminated before calling %s.', $functionName)); + } + } } diff --git a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php index f6b68d5884..6cb0af5ba3 100644 --- a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php +++ b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php @@ -272,6 +272,12 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase $this->assertTrue($process->isSuccessful()); } + public function testExitCodeTextIsNullWhenExitCodeIsNull() + { + $process = $this->getProcess(''); + $this->assertNull($process->getExitCodeText()); + } + public function testExitCodeText() { $process = $this->getProcess(''); @@ -512,6 +518,19 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase $this->assertLessThan($timeout + Process::TIMEOUT_PRECISION, $duration); } + public function testCheckTimeoutOnNonStartedProcess() + { + $process = $this->getProcess('php -r "sleep(3);"'); + $process->checkTimeout(); + } + + public function testCheckTimeoutOnTerminatedProcess() + { + $process = $this->getProcess('php -v'); + $process->run(); + $process->checkTimeout(); + } + public function testCheckTimeoutOnStartedProcess() { $timeout = 0.5; @@ -618,33 +637,52 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase } /** - * @expectedException \Symfony\Component\Process\Exception\LogicException - * @expectedExceptionMessage Process must be started before calling getOutput + * @dataProvider provideMethodsThatNeedARunningProcess */ - public function testGetOutputProcessNotStarted() + public function testMethodsThatNeedARunningProcess($method) { $process = $this->getProcess('php -m'); - $process->getOutput(); + $this->setExpectedException('Symfony\Component\Process\Exception\LogicException', sprintf('Process must be started before calling %s.', $method)); + call_user_func(array($process, $method)); + } + + public function provideMethodsThatNeedARunningProcess() + { + return array( + array('getOutput'), + array('getIncrementalOutput'), + array('getErrorOutput'), + array('getIncrementalErrorOutput'), + array('wait'), + ); } /** - * @expectedException \Symfony\Component\Process\Exception\LogicException - * @expectedExceptionMessage Process must be started before calling getErrorOutput + * @dataProvider provideMethodsThatNeedATerminatedProcess */ - public function testGetErrorOutputProcessNotStarted() + public function testMethodsThatNeedATerminatedProcess($method) { - $process = $this->getProcess('php -m'); - $process->getErrorOutput(); + $process = $this->getProcess('php -r "sleep(1);"'); + $process->start(); + try { + call_user_func(array($process, $method)); + $process->stop(0); + $this->fail('A LogicException must have been thrown'); + } catch (\Exception $e) { + $this->assertInstanceOf('Symfony\Component\Process\Exception\LogicException', $e); + $this->assertEquals(sprintf('Process must be terminated before calling %s.', $method), $e->getMessage()); + } + $process->stop(0); } - /** - * @expectedException \Symfony\Component\Process\Exception\LogicException - * @expectedExceptionMessage Process must be started before calling wait - */ - public function testWaitProcessWithoutTimeoutNotStarted() + public function provideMethodsThatNeedATerminatedProcess() { - $process = $this->getProcess('php -m')->setTimeout(null); - $process->wait(); + return array( + array('hasBeenSignaled'), + array('getTermSignal'), + array('hasBeenStopped'), + array('getStopSignal'), + ); } private function verifyPosixIsEnabled()