symfony · fabpot · Jan 25, 2016 · Jan 18, 2016
diff --git a/src/Symfony/Component/Process/Pipes/PipesInterface.php b/src/Symfony/Component/Process/Pipes/PipesInterface.php
@@ -53,6 +53,13 @@ public function readAndWrite($blocking, $close = false);
      */
     public function areOpen();
 
+    /**
+     * Returns if pipes are able to read output.
+     *
+     * @return bool
+     */
+    public function haveReadSupport();
+
     /**
      * Closes file handles and pipes.
      */

diff --git a/src/Symfony/Component/Process/Pipes/UnixPipes.php b/src/Symfony/Component/Process/Pipes/UnixPipes.php
@@ -27,13 +27,13 @@ class UnixPipes extends AbstractPipes
     /** @var bool */
     private $ptyMode;
     /** @var bool */
-    private $disableOutput;
+    private $haveReadSupport;
 
-    public function __construct($ttyMode, $ptyMode, $input, $disableOutput)
+    public function __construct($ttyMode, $ptyMode, $input, $haveReadSupport)
     {
         $this->ttyMode = (bool) $ttyMode;
         $this->ptyMode = (bool) $ptyMode;
-        $this->disableOutput = (bool) $disableOutput;
+        $this->haveReadSupport = (bool) $haveReadSupport;
 
         if (is_resource($input)) {
             $this->input = $input;
@@ -52,7 +52,7 @@ public function __destruct()
      */
     public function getDescriptors()
     {
-        if ($this->disableOutput) {
+        if (!$this->haveReadSupport) {
             $nullstream = fopen('/dev/null', 'c');
 
             return array(
@@ -191,6 +191,14 @@ public function readAndWrite($blocking, $close = false)
         return $read;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function haveReadSupport()
+    {
+        return $this->haveReadSupport;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -209,6 +217,6 @@ public function areOpen()
      */
     public static function create(Process $process, $input)
     {
-        return new static($process->isTty(), $process->isPty(), $input, $process->isOutputDisabled());
+        return new static($process->isTty(), $process->isPty(), $input, !$process->isOutputDisabled() || $process->hasCallback());
     }
 }
diff --git a/src/Symfony/Component/Process/Pipes/WindowsPipes.php b/src/Symfony/Component/Process/Pipes/WindowsPipes.php
@@ -36,13 +36,13 @@ class WindowsPipes extends AbstractPipes
         Process::STDERR => 0,
     );
     /** @var bool */
-    private $disableOutput;
+    private $haveReadSupport;
 
-    public function __construct($disableOutput, $input)
+    public function __construct($haveReadSupport, $input)
     {
-        $this->disableOutput = (bool) $disableOutput;
+        $this->haveReadSupport = (bool) $haveReadSupport;
 
-        if (!$this->disableOutput) {
+        if ($this->haveReadSupport) {
             // Fix for PHP bug #51800: reading from STDOUT pipe hangs forever on Windows if the output is too big.
             // Workaround for this problem is to use temporary files instead of pipes on Windows platform.
             //
@@ -76,7 +76,7 @@ public function __destruct()
      */
     public function getDescriptors()
     {
-        if ($this->disableOutput) {
+        if (!$this->haveReadSupport) {
             $nullstream = fopen('NUL', 'c');
 
             return array(
@@ -138,6 +138,14 @@ public function readAndWrite($blocking, $close = false)
         return $read;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function haveReadSupport()
+    {
+        return $this->haveReadSupport;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -168,7 +176,7 @@ public function close()
      */
     public static function create(Process $process, $input)
     {
-        return new static($process->isOutputDisabled(), $input);
+        return new static(!$process->isOutputDisabled() || $process->hasCallback(), $input);
     }
 
     /**

diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php
@@ -256,9 +256,6 @@ public function start(callable $callback = null)
         if ($this->isRunning()) {
             throw new RuntimeException('Process is already running');
         }
-        if ($this->outputDisabled && null !== $callback) {
-            throw new LogicException('Output has been disabled, enable it to allow the use of a callback.');
-        }
 
         $this->resetProcessData();
         $this->starttime = $this->lastOutputTime = microtime(true);
@@ -356,7 +353,12 @@ public function wait(callable $callback = null)
         $this->requireProcessIsStarted(__FUNCTION__);
 
         $this->updateStatus(false);
+
         if (null !== $callback) {
+            if (!$this->processPipes->haveReadSupport()) {
+                $this->stop(0);
+                throw new \LogicException('Pass the callback to the Process:start method or enableOutput to use a callback with Process::wait');
+            }
             $this->callback = $this->buildCallback($callback);
         }
 
@@ -1181,6 +1183,18 @@ public static function isPtySupported()
         return $result = (bool) @proc_open('echo 1', array(array('pty'), array('pty'), array('pty')), $pipes);
     }
 
+    /**
+     * Returns whether a callback is used on underlying process output.
+     *
+     * @internal
+     *
+     * @return bool
+     */
+    public function hasCallback()
+    {
+        return (bool) $this->callback;
+    }
+
     /**
      * Creates the descriptors needed by the proc_open.
      *
@@ -1194,7 +1208,7 @@ private function getDescriptors()
             $this->processPipes = UnixPipes::create($this, $this->input);
         }
 
-        return $this->processPipes->getDescriptors($this->outputDisabled);
+        return $this->processPipes->getDescriptors();
     }
 
     /**
@@ -1207,10 +1221,19 @@ private function getDescriptors()
      *
      * @return \Closure A PHP closure
      */
-    protected function buildCallback($callback)
+    protected function buildCallback(callable $callback = null)
     {
+        if ($this->outputDisabled) {
+            return function ($type, $data) use ($callback) {
+                if (null !== $callback) {
+                    call_user_func($callback, $type, $data);
+                }
+            };
+        }
+
         $out = self::OUT;
-        $callback = function ($type, $data) use ($callback, $out) {
+
+        return function ($type, $data) use ($callback, $out) {
             if ($out == $type) {
                 $this->addOutput($data);
             } else {

diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php
@@ -305,6 +305,19 @@ public function testCallbackIsExecutedForOutput()
         $this->assertTrue($called, 'The callback should be executed with the output');
     }
 
+    public function testCallbackIsExecutedForOutputWheneverOutputIsDisabled()
+    {
+        $p = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg('echo \'foo\';')));
+        $p->disableOutput();
+
+        $called = false;
+        $p->run(function ($type, $buffer) use (&$called) {
+            $called = $buffer === 'foo';
+        });
+
+        $this->assertTrue($called, 'The callback should be executed with the output');
+    }
+
     public function testGetErrorOutput()
     {
         $p = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg('$n = 0; while ($n < 3) { file_put_contents(\'php://stderr\', \'ERROR\'); $n++; }')));
@@ -1108,29 +1121,6 @@ public function testSetNullIdleTimeoutWhileOutputIsDisabled()
         $this->assertSame($process, $process->setIdleTimeout(null));
     }
 
-    /**
-     * @dataProvider provideStartMethods
-     */
-    public function testStartWithACallbackAndDisabledOutput($startMethod, $exception, $exceptionMessage)
-    {
-        $p = $this->getProcess('foo');
-        $p->disableOutput();
-        $this->setExpectedException($exception, $exceptionMessage);
-        if ('mustRun' === $startMethod) {
-            $this->skipIfNotEnhancedSigchild();
-        }
-        $p->{$startMethod}(function () {});
-    }
-
-    public function provideStartMethods()
-    {
-        return array(
-            array('start', 'Symfony\Component\Process\Exception\LogicException', 'Output has been disabled, enable it to allow the use of a callback.'),
-            array('run', 'Symfony\Component\Process\Exception\LogicException', 'Output has been disabled, enable it to allow the use of a callback.'),
-            array('mustRun', 'Symfony\Component\Process\Exception\LogicException', 'Output has been disabled, enable it to allow the use of a callback.'),
-        );
-    }
-
     /**
      * @dataProvider provideOutputFetchingMethods
      * @expectedException \Symfony\Component\Process\Exception\LogicException