From 200e67031556f25eb125669ba201373b8b017591 Mon Sep 17 00:00:00 2001 From: Tony Murray Date: Fri, 26 Aug 2016 01:50:29 -0500 Subject: [PATCH] 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 | 237 +++++++++++++++++++++++++++++++++++++++ check-services.php | 2 +- html/graph.php | 7 +- includes/rrdtool.inc.php | 136 ++++++++-------------- poll-billing.php | 2 +- poller.php | 2 +- 6 files changed, 294 insertions(+), 92 deletions(-) create mode 100644 LibreNMS/Proc.php diff --git a/LibreNMS/Proc.php b/LibreNMS/Proc.php new file mode 100644 index 0000000000..4c2050ffc9 --- /dev/null +++ b/LibreNMS/Proc.php @@ -0,0 +1,237 @@ +. + * + * @package LibreNMS + * @link http://librenms.org + * @copyright 2016 Tony Murray + * @author Tony Murray + */ + +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; + } +} diff --git a/check-services.php b/check-services.php index e418c8d385..d2aec7fc65 100755 --- a/check-services.php +++ b/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(); diff --git a/html/graph.php b/html/graph.php index 84ed9bfd3d..8411fca3dd 100644 --- a/html/graph.php +++ b/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); diff --git a/includes/rrdtool.inc.php b/includes/rrdtool.inc.php index a09a7e00b7..73a56a359f 100644 --- a/includes/rrdtool.inc.php +++ b/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,66 +47,46 @@ function rrdtool_initialize($dual_process = true) ); $cwd = $config['rrd_dir']; - $env = array(); - if(!is_resource($rrd_async_process)) { - $rrd_async_process = proc_open($command, $descriptor_spec, $rrd_async_pipes, $cwd, $env); - stream_set_blocking($rrd_async_pipes[1], false); - stream_set_blocking($rrd_async_pipes[2], false); + if (!rrdtool_running($rrd_sync_process)) { + $rrd_sync_process = new Proc($command, $descriptor_spec, $cwd); } - if ($dual_process && !is_resource($rrd_sync_process)) { - $rrd_sync_process = proc_open($command, $descriptor_spec, $rrd_sync_pipes, $cwd, $env); - stream_set_blocking($rrd_sync_pipes[1], false); - stream_set_blocking($rrd_sync_pipes[2], false); + if ($dual_process && !rrdtool_running($rrd_async_process)) { + $rrd_async_process = new Proc($command, $descriptor_spec, $cwd); + $rrd_async_process->setSynchronous(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() { - 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) +function rrdtool_close() { - global $vdebug; - if ($vdebug) { - d_echo(stream_get_contents($rrd_pipes[1])); - d_echo(stream_get_contents($rrd_pipes[2])); + global $rrd_sync_process, $rrd_async_process; + + if (rrdtool_running($rrd_sync_process)) { + $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 * Opens its own rrdtool pipe. @@ -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']) { - $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"); - } + $cmd = rrdtool_build_command('graph', $graph_file, $options); - fclose($rrd_async_pipes[0]); - - $line = ""; - $data = ""; - while (strlen($line) < 1) { - $line = fgets($rrd_async_pipes[1], 1024); - $data .= $line; - } - - $return_value = rrdtool_terminate(); + $output = implode($rrd_sync_process->sendCommand($cmd)); if ($debug) { - echo '

'; - echo "graph $graph_file $options"; - - echo '

'; - echo "command returned $return_value ($data)\n"; - echo '

'; + echo "

$cmd

"; + echo "

command returned ($output)

"; } - 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 - * @global $debug - * @global $rrd_pipes + * @throws FileExistsException thrown when a create command is set to rrdtool < 1.4 and the rrd already exists + * @throws Exception thrown when the rrdtool process(s) cannot be started */ 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) { - fwrite($rrd_sync_pipes[0], $cmd . "\n"); - - // this causes us to block until we receive output for up to the timeout in seconds - stream_select($r = $rrd_sync_pipes, $w = null, $x = null, 10); - $output = array(stream_get_contents($rrd_sync_pipes[1]), stream_get_contents($rrd_sync_pipes[2])); - + if ($command == 'last' && rrdtool_initialize(false)) { + // send this to our synchronous process so output is guaranteed + $output = $rrd_sync_process->sendCommand($cmd); + } elseif (rrdtool_initialize()) { + // don't care about the return of other commands, so send them to the faster async process + $output = $rrd_async_process->sendCommand($cmd); } else { - fwrite($rrd_async_pipes[0], $cmd . "\n"); - $output = array(stream_get_contents($rrd_async_pipes[1]), stream_get_contents($rrd_async_pipes[2])); + throw new Exception('rrdtool could not start'); } 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); } diff --git a/poll-billing.php b/poll-billing.php index f1a29d4c5c..55ece83b25 100755 --- a/poll-billing.php +++ b/poll-billing.php @@ -158,4 +158,4 @@ if ($poller_time > 300) { } echo "\nCompleted in $poller_time sec\n"; -rrdtool_terminate(); \ No newline at end of file +rrdtool_close(); diff --git a/poller.php b/poller.php index 5a6d5b47dc..0868dde869 100755 --- a/poller.php +++ b/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());