Fix defunct process issues: create Proc class (#4210)

* Proc class
Encapsulate processes in a class, when they go out of scope, we can use the __destruct() process to make sure the process terminates.

* Fix rrdtool_last checks failing

* Don't close rrdtool in rrd_graph
Try to start rrdtool process if they haven't been started yet
Remove some extra debug output

* phpdocs added
several cleanups
should still be functioning the same
Only open one  rrdtool process for graph.php

LibreNMS/Proc.php

@@ -0,0 +1,237 @@
+<?php
+/**
+ * Proc.php
+ *
+ * Executes a process with proc_open() and guarantees it is terminated on exit
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @package    LibreNMS
+ * @link       http://librenms.org
+ * @copyright  2016 Tony Murray
+ * @author     Tony Murray <murraytony@gmail.com>
+ */
+
+namespace LibreNMS;
+
+use Exception;
+
+class Proc
+{
+    /**
+     * @var resource the process this object is responsible for
+     */
+    private $_process;
+    /**
+     * @var array array of process pipes [stdin,stdout,stderr]
+     */
+    private $_pipes;
+
+    /**
+     * @var bool if this process is synchronous (waits for output)
+     */
+    private $_synchronous;
+
+    /**
+     * Create and run a new process
+     * Most arguments match proc_open()
+     *
+     * @param string $cmd the command to execute
+     * @param array $descriptorspec the definition of pipes to initialize
+     * @param null $cwd working directory to change to
+     * @param array|null $env array of environment variables to set
+     * @param bool $blocking set the output pipes to blocking (default: false)
+     * @throws Exception the command was unable to execute
+     */
+    public function __construct($cmd, $descriptorspec, $cwd = null, $env = null, $blocking = false)
+    {
+        $this->_process = proc_open($cmd, $descriptorspec, $this->_pipes, $cwd, $env);
+        if (!is_resource($this->_process)) {
+            throw new Exception("Command failed: $cmd");
+        }
+        stream_set_blocking($this->_pipes[1], $blocking);
+        stream_set_blocking($this->_pipes[2], $blocking);
+        $this->_synchronous = true;
+    }
+
+    /**
+     * Called when this object goes out of scope or php exits
+     * If it is still running, terminate the process
+     */
+    public function __destruct()
+    {
+        if ($this->isRunning()) {
+            $this->terminate();
+        }
+    }
+
+    /**
+     * Get one of the pipes
+     * 0 - stdin
+     * 1 - stdout
+     * 2 - stderr
+     *
+     * @param int $nr pipe number (0-2)
+     * @return resource the pipe handle
+     */
+    public function pipe($nr)
+    {
+        return $this->_pipes[$nr];
+    }
+
+
+    /**
+     * Send a command to this process and return the output
+     * the output may not correspond to this command if this
+     * process is not synchronous
+     * If the command isn't terminated with a newline, add one
+     *
+     * @param $command
+     * @return array
+     */
+    public function sendCommand($command)
+    {
+        if (!ends_with($command, PHP_EOL)) {
+            $command .= PHP_EOL;
+        }
+        $this->sendInput($command);
+
+        return $this->getOutput();
+    }
+
+    /**
+     * Send data to stdin
+     *
+     * @param string $data the string to send
+     */
+    public function sendInput($data)
+    {
+        fwrite($this->_pipes[0], $data);
+    }
+
+    /**
+     * Gets the current output of the process
+     * If this process is set to synchronous, wait for output
+     *
+     * @param int $timeout time to wait for output, only applies if this process is synchronous
+     * @return array [stdout, stderr]
+     */
+    public function getOutput($timeout = 15)
+    {
+        if ($this->_synchronous) {
+            $pipes = array($this->_pipes[1], $this->_pipes[2]);
+            $w = null;
+            $x = null;
+
+            stream_select($pipes, $w, $x, $timeout);
+        }
+        return array(stream_get_contents($this->_pipes[1]), stream_get_contents($this->_pipes[2]));
+    }
+
+    /**
+     * Attempt to gracefully close this process
+     * optionally send one last piece of input
+     * such as a quit command
+     *
+     * @param string $cmd the final command to send
+     * @return int the exit status of this process (-1 means error)
+     */
+    public function close($cmd = null)
+    {
+        if (isset($cmd)) {
+            $this->sendInput($cmd);
+        }
+
+        fclose($this->_pipes[0]);
+        fclose($this->_pipes[1]);
+        fclose($this->_pipes[2]);
+
+        return proc_close($this->_process);
+    }
+
+    /**
+     * Forcibly close this process
+     * Please attempt to run close() instead of this
+     * This will be called when this object is destroyed if the process is still running
+     *
+     * @param int $signal the signal to send
+     * @throws Exception
+     */
+    public function terminate($signal = 15)
+    {
+        $status = $this->getStatus();
+
+        fclose($this->_pipes[1]);
+        fclose($this->_pipes[2]);
+
+        $closed = proc_terminate($this->_process, $signal);
+
+        if (!$closed) {
+            // try harder
+            $pid = $status['pid'];
+            $killed = posix_kill($pid, 9); //9 is the SIGKILL signal
+            proc_close($this->_process);
+
+            if (!$killed) {
+                throw new Exception("Terminate failed!");
+            }
+        }
+    }
+
+    /**
+     * Get the status of this process
+     * see proc_get_status()
+     *
+     * @return array status array
+     */
+    public function getStatus()
+    {
+        return proc_get_status($this->_process);
+    }
+
+    /**
+     * Check if this process is running
+     *
+     * @return bool
+     */
+    public function isRunning()
+    {
+        if(!is_resource($this->_process)) {
+            return false;
+        }
+        $st = $this->getStatus();
+        return isset($st['running']);
+    }
+
+    /**
+     * If this process waits for output
+     * @return boolean
+     */
+    public function isSynchronous()
+    {
+        return $this->_synchronous;
+    }
+
+    /**
+     * Set this process as synchronous, by default processes are synchronous
+     * It is advisable not to change this mid way as output could get mixed up
+     * or you could end up blocking until the getOutput timeout expires
+     *
+     * @param boolean $synchronous
+     */
+    public function setSynchronous($synchronous)
+    {
+        $this->_synchronous = $synchronous;
+    }
+}

check-services.php

@@ -53,4 +53,4 @@ foreach (dbFetchRows('SELECT * FROM `devices` AS D, `services` AS S WHERE S.devi
     poll_service($service);
 
 } //end foreach
-rrdtool_terminate();
+rrdtool_close();

html/graph.php

@@ -32,9 +32,9 @@ if (isset($_GET['debug'])) {
 
 require_once '../includes/defaults.inc.php';
 require_once '../config.php';
+require_once '../includes/common.php';
 require_once '../includes/definitions.inc.php';
 
-require_once '../includes/common.php';
 require_once '../includes/dbFacile.php';
 require_once '../includes/rewrites.php';
 require_once 'includes/functions.inc.php';
@@ -42,9 +42,12 @@ require_once '../includes/rrdtool.inc.php';
 if ($config['allow_unauth_graphs'] != true) {
     require_once 'includes/authenticate.inc.php';
 }
+
+rrdtool_initialize(false);
+
 require 'includes/graphs/graph.inc.php';
 
-$console_color = new Console_Color2();
+rrdtool_close();
 
 $end = microtime(true);
 $run = ($end - $start);

includes/rrdtool.inc.php

@@ -2,7 +2,7 @@
 /**
  * rrdtool.inc.php
  *
- * Helper for processing rrd requests efficiently
+ * Helper for processing rrdtool requests efficiently
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -26,6 +26,7 @@
  */
 
 use LibreNMS\Exceptions\FileExistsException;
+use LibreNMS\Proc;
 
 /**
  * Opens up a pipe to RRDTool using handles provided
@@ -35,7 +36,7 @@ use LibreNMS\Exceptions\FileExistsException;
  */
 function rrdtool_initialize($dual_process = true)
 {
-    global $config, $rrd_async_process, $rrd_async_pipes, $rrd_sync_process, $rrd_sync_pipes;
+    global $config, $rrd_sync_process, $rrd_async_process;
 
     $command = $config['rrdtool'] . ' -';
 
@@ -46,65 +47,45 @@ function rrdtool_initialize($dual_process = true)
     );
 
     $cwd = $config['rrd_dir'];
-    $env = array();
 
-    if(!is_resource($rrd_async_process)) {
+    if (!rrdtool_running($rrd_sync_process)) {
-        $rrd_async_process = proc_open($command, $descriptor_spec, $rrd_async_pipes, $cwd, $env);
+        $rrd_sync_process = new Proc($command, $descriptor_spec, $cwd);
-        stream_set_blocking($rrd_async_pipes[1], false);
-        stream_set_blocking($rrd_async_pipes[2], false);
     }
 
-    if ($dual_process && !is_resource($rrd_sync_process)) {
+    if ($dual_process && !rrdtool_running($rrd_async_process)) {
-        $rrd_sync_process = proc_open($command, $descriptor_spec, $rrd_sync_pipes, $cwd, $env);
+        $rrd_async_process = new Proc($command, $descriptor_spec, $cwd);
-        stream_set_blocking($rrd_sync_pipes[1], false);
+        $rrd_async_process->setSynchronous(false);
-        stream_set_blocking($rrd_sync_pipes[2], false);
     }
 
-    return is_resource($rrd_async_process) && ($dual_process ? is_resource($rrd_sync_process) : true);
+    return rrdtool_running($rrd_sync_process) && ($dual_process ? rrdtool_running($rrd_async_process) : true);
+}
+
+/**
+ * Checks if the variable is a running rrdtool process
+ *
+ * @param $process
+ * @return bool
+ */
+function rrdtool_running(&$process)
+{
+    return isset($process) && $process instanceof Proc && $process->isRunning();
 }
 
 /**
  * Close all open rrdtool processes.
  * This should be done before exiting a script that has called rrdtool_initilize()
- *
- * @return bool indicates success
  */
-function rrdtool_terminate() {
+function rrdtool_close()
-    global $rrd_async_process, $rrd_async_pipes, $rrd_sync_process, $rrd_sync_pipes;
-
-    $ret = rrdtool_pipe_close($rrd_async_process, $rrd_async_pipes);
-    if ($rrd_sync_pipes) {
-        $ret = rrdtool_pipe_close($rrd_sync_process, $rrd_sync_pipes) && $ret;
-    }
-
-    return $ret;
-}
-
-/**
- * Closes the pipe to RRDTool
- *
- * @internal
- * @param  resource $rrd_process
- * @param  array $rrd_pipes
- * @return integer
- */
-function rrdtool_pipe_close($rrd_process, &$rrd_pipes)
 {
-    global $vdebug;
+    global $rrd_sync_process, $rrd_async_process;
-    if ($vdebug) {
+
-        d_echo(stream_get_contents($rrd_pipes[1]));
+    if (rrdtool_running($rrd_sync_process)) {
-        d_echo(stream_get_contents($rrd_pipes[2]));
+        $rrd_sync_process->close('quit');
+    }
+    if (rrdtool_running($rrd_async_process)) {
+        $rrd_async_process->close('quit');
     }
-
-    fclose($rrd_pipes[0]);
-    fclose($rrd_pipes[1]);
-    fclose($rrd_pipes[2]);
-
-    // It is important that you close any pipes before calling
-    // proc_close in order to avoid a deadlock
-    return proc_terminate($rrd_process);
 }
-
 
 /**
  * Generates a graph file at $graph_file using $options
@@ -116,37 +97,19 @@ function rrdtool_pipe_close($rrd_process, &$rrd_pipes)
  */
 function rrdtool_graph($graph_file, $options)
 {
-    global $config, $debug, $rrd_async_pipes;
+    global $debug, $rrd_sync_process;
 
     if (rrdtool_initialize(false)) {
-        if ($config['rrdcached']) {
+        $cmd = rrdtool_build_command('graph', $graph_file, $options);
-            $options = str_replace(array($config['rrd_dir'].'/', $config['rrd_dir']), '', $options);
-            fwrite($rrd_async_pipes[0], 'graph --daemon ' . $config['rrdcached'] . " $graph_file $options");
-        } else {
-            fwrite($rrd_async_pipes[0], "graph $graph_file $options");
-        }
 
-        fclose($rrd_async_pipes[0]);
+        $output = implode($rrd_sync_process->sendCommand($cmd));
-
-        $line = "";
-        $data = "";
-        while (strlen($line) < 1) {
-            $line = fgets($rrd_async_pipes[1], 1024);
-            $data .= $line;
-        }
-
-        $return_value = rrdtool_terminate();
 
         if ($debug) {
-            echo '<p>';
+            echo "<p>$cmd</p>";
-            echo "graph $graph_file $options";
+            echo "<p>command returned ($output)</p>";
-
-            echo '</p><p>';
-            echo "command returned $return_value ($data)\n";
-            echo '</p>';
         }
 
-        return $data;
+        return $output;
     } else {
         return 0;
     }
@@ -161,13 +124,12 @@ function rrdtool_graph($graph_file, $options)
  * @param string $filename The full patth to the rrd file
  * @param string $options rrdtool command options
  * @return array the output of stdout and stderr in an array
- * @global $config
+ * @throws FileExistsException thrown when a create command is set to rrdtool < 1.4 and the rrd already exists
- * @global $debug
+ * @throws Exception thrown when the rrdtool process(s) cannot be started
- * @global $rrd_pipes
  */
 function rrdtool($command, $filename, $options)
 {
-    global $config, $debug, $vdebug, $rrd_async_pipes, $rrd_sync_pipes;
+    global $config, $debug, $vdebug, $rrd_async_process, $rrd_sync_process;
 
     try {
         $cmd = rrdtool_build_command($command, $filename, $options);
@@ -180,22 +142,20 @@ function rrdtool($command, $filename, $options)
 
     // do not write rrd files, but allow read-only commands
     $ro_commands = array('graph', 'graphv', 'dump', 'fetch', 'first', 'last', 'lastupdate', 'info', 'xport');
-    if ($config['norrd'] && !in_array($command, $ro_commands)) {
+    if (!empty($config['norrd']) && !in_array($command, $ro_commands)) {
         c_echo('[%rRRD Disabled%n]');
         return array(null, null);
     }
 
     // send the command!
-    if($command == 'last' && $rrd_sync_pipes) {
+    if ($command == 'last' && rrdtool_initialize(false)) {
-        fwrite($rrd_sync_pipes[0], $cmd . "\n");
+        // send this to our synchronous process so output is guaranteed
-
+        $output = $rrd_sync_process->sendCommand($cmd);
-        // this causes us to block until we receive output for up to the timeout in seconds
+    } elseif (rrdtool_initialize()) {
-        stream_select($r = $rrd_sync_pipes, $w = null, $x = null, 10);
+        // don't care about the return of other commands, so send them to the faster async process
-        $output = array(stream_get_contents($rrd_sync_pipes[1]), stream_get_contents($rrd_sync_pipes[2]));
+        $output = $rrd_async_process->sendCommand($cmd);
-
     } else {
-        fwrite($rrd_async_pipes[0], $cmd . "\n");
+        throw new Exception('rrdtool could not start');
-        $output = array(stream_get_contents($rrd_async_pipes[1]), stream_get_contents($rrd_async_pipes[2]));
     }
 
     if ($vdebug) {
@@ -241,6 +201,7 @@ function rrdtool_build_command($command, $filename, $options)
     ) {
         // only relative paths if using rrdcached
         $filename = str_replace(array($config['rrd_dir'].'/', $config['rrd_dir']), '', $filename);
+        $options = str_replace(array($config['rrd_dir'].'/', $config['rrd_dir']), '', $options);
 
         return "$command $filename $options --daemon " . $config['rrdcached'];
     }
@@ -347,7 +308,8 @@ function rrd_name($host, $extra, $extension = ".rrd")
  * @param $vmport
  * @return string full path to the rrd.
  */
-function proxmox_rrd_name($pmxcluster, $vmid, $vmport) {
+function proxmox_rrd_name($pmxcluster, $vmid, $vmport)
+{
     global $config;
 
     $pmxcdir = join('/', array($config['rrd_dir'], 'proxmox', safename($pmxcluster)));
@@ -422,7 +384,7 @@ function rrdtool_data_update($device, $measurement, $tags, $fields)
     $rrd_name = $tags['rrd_name'] ?: $measurement;
     $step = $tags['rrd_step'] ?: 300;
     $oldname = $tags['rrd_oldname'];
-    if (isset($oldname) && !empty($oldname)) {
+    if (!empty($oldname)) {
         rrd_file_rename($device, $oldname, $rrd_name);
     }
 

poll-billing.php

@@ -158,4 +158,4 @@ if ($poller_time > 300) {
 }
 echo "\nCompleted in $poller_time sec\n";
 
-rrdtool_terminate();
+rrdtool_close();

poller.php

@@ -158,7 +158,7 @@ echo ("\n".'MySQL: Cell['.($db_stats['fetchcell'] + 0).'/'.round(($db_stats['fet
 echo "\n";
 
 logfile($string);
-rrdtool_terminate();
+rrdtool_close();
 unset($config);
 // Remove this for testing
 // print_r(get_defined_vars());