Skip to content

Fixed system interpreter leaked issue #1134.#1135

Closed
markwu wants to merge 3 commits into
phpbrew:masterfrom
markwu:fix-system-interpreter-leaked-issue
Closed

Fixed system interpreter leaked issue #1134.#1135
markwu wants to merge 3 commits into
phpbrew:masterfrom
markwu:fix-system-interpreter-leaked-issue

Conversation

@markwu
Copy link
Copy Markdown
Contributor

@markwu markwu commented Dec 28, 2019

Use exec() to run php code with used interpreter instead of system interpreter.

Use exec() to run php code with used interpreter instead of system interpreter.
@markwu
Copy link
Copy Markdown
Contributor Author

markwu commented Dec 28, 2019

The result:

$ phpbrew list
* php-7.4.1
  php-7.3.13
  php-5.6.40

$ phpbrew system
/Users/mark/.phpbrew/php/php-7.3.13/bin/php

$ phpbrew list-ini
Loaded ini files:
 - /Users/mark/.phpbrew/php/php-7.4.1/var/db/ast.ini
 - /Users/mark/.phpbrew/php/php-7.4.1/var/db/gd.ini
 - /Users/mark/.phpbrew/php/php-7.4.1/var/db/opcache.ini
 - /Users/mark/.phpbrew/php/php-7.4.1/var/db/xdebug.ini

@markwu
Copy link
Copy Markdown
Contributor Author

markwu commented Dec 28, 2019

phpbrew info has the same problem, too.

@markwu
Copy link
Copy Markdown
Contributor Author

markwu commented Dec 28, 2019

I try to wrap the function, so I can get the function call through used php interpreter like following. We can get exact the same result as before.

UsePhpFunctionWrapper::execute('php_ini_loaded_file()', $output) ? $file = $output : $file = '';
<?php

namespace PhpBrew;
use PhpBrew\Config;

class UsePhpFunctionWrapper
{
    public static function execute($func, &$output = null)
    {
        $retVal = false;

        $command = Config::getCurrentPhpBin() . '/php';

        $descriptorspec = array(
           0 => array("pipe", "r"),
           1 => array("pipe", "w"),
           2 => array("pipe", "w")
        );

        $process = proc_open($command, $descriptorspec, $pipes);

        if (is_resource($process)) {
            fwrite($pipes[0], '<?php echo serialize(' . $func  .'); ?>');
            fclose($pipes[0]);

            $output = stream_get_contents($pipes[1]);
            fclose($pipes[1]);

            $retVal = proc_close($process);

            if ($retVal === 0) {
                $output = unserialize($output);

                return true;
            }
        }

        return false;
    }
}

@markwu
Copy link
Copy Markdown
Contributor Author

markwu commented Dec 28, 2019

phpbrew config, phpbrew list-ini and phpbrew info fixed.

@morozov
Copy link
Copy Markdown
Contributor

morozov commented Dec 28, 2019

This looks even more complicated than I thought it would. We can do it much simpler:

  1. phpbrew config could be implemented as:
    # Bash
    $ command $EDITOR "$(php -r "echo php_ini_loaded_file();")"
    
    # Fish
    $ command $EDITOR (php -r "echo php_ini_loaded_file();")
  2. phpbrew info currently excutes a series of subsequent calls to the underlying PHP and glues the output together. Instead, we could do something like:
    # phpbrew info-script generates some script
    $ phpbrew info-script
    <?php
    
    echo 'Version' . PHP_EOL;
    echo 'PHP-' . phpversion() . PHP_EOL;
    // and so on
    
    # its output gets piped to the currently used PHP
    phpbrew info-script | php

@markwu
Copy link
Copy Markdown
Contributor Author

markwu commented Dec 28, 2019

Yes, you are right, it's much simpler. I just tried to keep everything as before.

But, I am not so familiar with the shell scripts. I will leave the implement for you, and see If I can catch how you do it.

@morozov
Copy link
Copy Markdown
Contributor

morozov commented Dec 29, 2019

Closing in favor of #1136.

@morozov morozov closed this Dec 29, 2019
@morozov morozov self-assigned this Dec 29, 2019
@morozov morozov added Bash Fish Regression A bug that that was introduced as part of a feature or another bug fix labels Dec 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bash Fish Regression A bug that that was introduced as part of a feature or another bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants