Skip to content

Commit b08e5e2

Browse files
TheSyscallTheSyscall
committed
Use ShellCommand abstraction
1 parent 322bbeb commit b08e5e2

3 files changed

Lines changed: 92 additions & 140 deletions

File tree

‎library/Pdfexport/Backend/HeadlessChromeBackend.php‎

Lines changed: 23 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Icinga\File\Storage\StorageInterface;
1515
use Icinga\File\Storage\TemporaryLocalFileStorage;
1616
use Icinga\Module\Pdfexport\PrintableHtmlDocument;
17+
use Icinga\Module\Pdfexport\ShellCommand;
1718
use LogicException;
1819
use Throwable;
1920
use WebSocket\Client;
@@ -23,21 +24,6 @@ class HeadlessChromeBackend implements PfdPrintBackend
2324
/** @var int */
2425
public const MIN_SUPPORTED_CHROME_VERSION = 59;
2526

26-
/** @var int */
27-
protected const CHROME_START_MAX_WAIT_TIME = 10;
28-
29-
/** @var int */
30-
protected const CHROME_CLOSE_MAX_WAIT_TIME = 5;
31-
32-
/** @var int */
33-
protected const PROCESS_IDLE_TIME = 100000;
34-
35-
/** @var int */
36-
protected const STREAM_WAIT_TIME = 200000;
37-
38-
/** @var int */
39-
protected const STREAM_CHUNK_SIZE = 8192;
40-
4127
/**
4228
* Line of stderr output identifying the websocket url
4329
*
@@ -62,9 +48,7 @@ class HeadlessChromeBackend implements PfdPrintBackend
6248

6349
private array $interceptedEvents = [];
6450

65-
protected $process;
66-
67-
protected array $pipes = [];
51+
protected ?ShellCommand $process = null;
6852

6953
protected ?string $socket = null;
7054

@@ -111,11 +95,6 @@ public static function createLocal(string $path, bool $useFile = false): static
11195
}
11296

11397
$browserHome = $instance->getFileStorage()->resolvePath('HOME');
114-
$descriptors = [
115-
0 => ['pipe', 'r'], // stdin
116-
1 => ['pipe', 'w'], // stdout
117-
2 => ['pipe', 'w'], // stderr
118-
];
11998

12099
$commandLine = join(' ', [
121100
escapeshellarg($path),
@@ -141,58 +120,25 @@ public static function createLocal(string $path, bool $useFile = false): static
141120
Logger::debug('Starting browser process: %s', $commandLine);
142121
}
143122

144-
$instance->process = proc_open($commandLine, $descriptors, $instance->pipes, null, $env);
145-
146-
if (! is_resource($instance->process)) {
147-
throw new Exception('Could not start browser process.');
148-
}
149-
150-
// Non-blocking mode
151-
stream_set_blocking($instance->pipes[2], false);
152-
153-
$startTime = time();
154-
155-
while (true) {
156-
$status = proc_get_status($instance->process);
157-
158-
// Timeout handling
159-
if ((time() - $startTime) > self::CHROME_START_MAX_WAIT_TIME) {
160-
proc_terminate($instance->process, 6); // SIGABRT
161-
Logger::error(
162-
'Browser timed out after %d seconds without the expected output',
163-
self::CHROME_CLOSE_MAX_WAIT_TIME,
164-
);
165-
166-
throw new Exception(
167-
'Received empty response or none at all from browser.'
168-
. ' Please check the logs for further details.',
169-
);
123+
$instance->process = new ShellCommand($commandLine, false, $env);
124+
$instance->process->start();
125+
Logger::debug('Started browser process');
126+
$instance->process->wait(function ($stdout, $stderr) use ($instance) {
127+
if ($stdout !== '') {
128+
Logger::debug('Caught browser stdout: %d', mb_strlen($stdout));
170129
}
171-
172-
$read = [$instance->pipes[2]];
173-
$write = null;
174-
$except = null;
175-
176-
if (stream_select($read, $write, $except, 0, self::STREAM_WAIT_TIME)) {
177-
$chunk = fread($instance->pipes[2], self::STREAM_CHUNK_SIZE);
178-
179-
if ($chunk !== false && $chunk !== '') {
180-
Logger::debug('Caught browser output: %s', $chunk);
181-
182-
if (preg_match(self::DEBUG_ADDR_PATTERN, trim($chunk), $matches)) {
183-
$instance->socket = $matches[1];
184-
$instance->browserId = $matches[2];
185-
break;
186-
}
130+
if ($stderr !== '') {
131+
Logger::error('Browser process stderr: %d', mb_strlen($stderr));
132+
if (preg_match(self::DEBUG_ADDR_PATTERN, trim($stderr), $matches)) {
133+
$instance->socket = $matches[1];
134+
$instance->browserId = $matches[2];
135+
136+
Logger::debug('Caught browser info socket: %s, id: %s', $instance->socket, $instance->browserId);
137+
return false;
187138
}
188139
}
189-
190-
if (! $status['running']) {
191-
break;
192-
}
193-
194-
usleep(self::PROCESS_IDLE_TIME);
195-
}
140+
return true;
141+
});
196142

197143
if ($instance->socket === null || $instance->browserId === null) {
198144
throw new Exception('Could not start browser process.');
@@ -203,35 +149,11 @@ public static function createLocal(string $path, bool $useFile = false): static
203149

204150
protected function closeLocal(): void
205151
{
206-
foreach ($this->pipes as $pipe) {
207-
fclose($pipe);
208-
}
209-
$this->pipes = [];
152+
Logger::debug('Closing local chrome instance');
210153

211154
if ($this->process !== null) {
212-
proc_terminate($this->process);
213-
214-
$start = time();
215-
$running = true;
216-
217-
while ($running && (time() - $start) < self::CHROME_CLOSE_MAX_WAIT_TIME) {
218-
$status = proc_get_status($this->process);
219-
$running = $status['running'];
220-
221-
if ($running) {
222-
usleep(self::PROCESS_IDLE_TIME);
223-
}
224-
}
225-
226-
// If still running after wait time seconds, force kills the entire process group
227-
if ($running) {
228-
$status = proc_get_status($this->process);
229-
if (! empty($status['pid'])) {
230-
posix_kill(-$status['pid'], SIGKILL);
231-
}
232-
}
233-
234-
proc_close($this->process);
155+
$code = $this->process->stop();
156+
Logger::error("Closed local chrome with exit code %d", $code);
235157
$this->process = null;
236158
}
237159

@@ -356,7 +278,7 @@ public function getPage(): Client
356278

357279
try {
358280
$this->communicate($this->page, 'Console.enable');
359-
} catch (Exception $_) {
281+
} catch (Exception) {
360282
// Deprecated, might fail
361283
}
362284
}
@@ -526,7 +448,7 @@ private function parseApiResponse(string $payload)
526448
}
527449
}
528450

529-
private function registerEvent($method, $params)
451+
private function registerEvent($method, $params): void
530452
{
531453
if (Logger::getInstance()->getLevel() === Logger::DEBUG) {
532454
$shortenValues = function ($params) use (&$shortenValues) {

‎library/Pdfexport/BackendLocator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ protected function connectToLocalChrome(string $section): ?PfdPrintBackend
122122
$binary,
123123
Config::module('pdfexport')->get('chrome', 'force_temp_storage', '0') === '1',
124124
);
125-
Logger::info("Connected WebDriver Backend: $section");
125+
Logger::info("Connected local chrome Backend: $section");
126126
return $backend;
127127
} catch (Exception $e) {
128128
Logger::warning(

‎library/Pdfexport/ShellCommand.php‎

Lines changed: 68 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,50 @@
55

66
namespace Icinga\Module\Pdfexport;
77

8+
use Exception;
9+
810
class ShellCommand
911
{
1012
/** @var string Command to execute */
11-
protected $command;
13+
protected string $command;
14+
15+
/** @var ?int Exit code of the command */
16+
protected ?int $exitCode = null;
1217

13-
/** @var int Exit code of the command */
14-
protected $exitCode;
18+
/** @var array|null Environment variables */
19+
protected ?array $env;
1520

1621
/** @var ?resource Process resource */
1722
protected $resource;
1823

24+
/** @var object|null Named pipe resources */
25+
protected ?object $namedPipes;
26+
27+
/** @var string collected stdout */
28+
protected string $stdout;
29+
30+
/** @var string collected stderr */
31+
protected string $stderr;
32+
1933
/**
2034
* Create a new command
2135
*
2236
* @param string $command The command to execute
2337
* @param bool $escape Whether to escape the command
38+
* @param array|null $env Environment variables
2439
*/
25-
public function __construct($command, $escape = true)
40+
public function __construct(string $command, bool $escape = true, ?array $env = null)
2641
{
27-
$command = (string) $command;
28-
2942
$this->command = $escape ? escapeshellcmd($command) : $command;
43+
$this->env = $env;
3044
}
3145

3246
/**
3347
* Get the exit code of the command
3448
*
3549
* @return int
3650
*/
37-
public function getExitCode()
51+
public function getExitCode(): int
3852
{
3953
return $this->exitCode;
4054
}
@@ -44,7 +58,7 @@ public function getExitCode()
4458
*
4559
* @return object
4660
*/
47-
public function getStatus()
61+
public function getStatus(): object
4862
{
4963
$status = (object) proc_get_status($this->resource);
5064
if ($status->running === false && $this->exitCode === null) {
@@ -56,64 +70,67 @@ public function getStatus()
5670
return $status;
5771
}
5872

59-
/**
60-
* Execute the command
61-
*
62-
* @return object
63-
*
64-
* @throws \Exception
65-
*/
66-
public function execute()
73+
public function start(): void
6774
{
6875
if ($this->resource !== null) {
69-
throw new \Exception('Command already started');
76+
throw new Exception('Command already started');
7077
}
7178

7279
$descriptors = [
7380
['pipe', 'r'], // stdin
7481
['pipe', 'w'], // stdout
75-
['pipe', 'w'], // stderr
82+
['pipe', 'w'], // stderr
7683
];
7784

7885
$this->resource = proc_open(
7986
$this->command,
8087
$descriptors,
8188
$pipes,
89+
null,
90+
$this->env,
8291
);
8392

8493
if (! is_resource($this->resource)) {
85-
throw new \Exception(sprintf(
94+
throw new Exception(sprintf(
8695
"Can't fork '%s'",
8796
$this->command,
8897
));
8998
}
9099

91-
$namedpipes = (object) [
92-
'stdin' => &$pipes[0],
100+
$this->namedPipes = (object) [
101+
'stdin' => &$pipes[0],
93102
'stdout' => &$pipes[1],
94103
'stderr' => &$pipes[2],
95104
];
96105

97-
fclose($namedpipes->stdin);
106+
fclose($this->namedPipes->stdin);
107+
}
108+
109+
public function wait($callback = null): void
110+
{
111+
if ($this->resource === null) {
112+
throw new Exception('Command not started');
113+
}
98114

99-
$read = [$namedpipes->stderr, $namedpipes->stdout];
115+
$read = [$this->namedPipes->stderr, $this->namedPipes->stdout];
100116
$origRead = $read;
101-
$write = null; // stdin not handled
117+
// stdin not handled
118+
$write = null;
102119
$except = null;
103-
$stdout = '';
104-
$stderr = '';
120+
$this->stdout = '';
121+
$this->stderr = '';
105122

106-
stream_set_blocking($namedpipes->stdout, false); // non-blocking
107-
stream_set_blocking($namedpipes->stderr, false);
123+
stream_set_blocking($this->namedPipes->stdout, false);
124+
stream_set_blocking($this->namedPipes->stderr, false);
108125

109126
while (stream_select($read, $write, $except, 0, 20000) !== false) {
110127
foreach ($read as $pipe) {
111-
if ($pipe === $namedpipes->stdout) {
112-
$stdout .= stream_get_contents($pipe);
128+
if ($pipe === $this->namedPipes->stdout) {
129+
$this->stdout .= stream_get_contents($pipe);
113130
}
114131

115-
if ($pipe === $namedpipes->stderr) {
116-
$stderr .= stream_get_contents($pipe);
132+
if ($pipe === $this->namedPipes->stderr) {
133+
$this->stderr .= stream_get_contents($pipe);
117134
}
118135
}
119136

@@ -129,21 +146,34 @@ public function execute()
129146

130147
// Reset pipes
131148
$read = $origRead;
149+
150+
if ($callback !== null) {
151+
$continue = call_user_func($callback, $this->stdout, $this->stderr);
152+
if ($continue === false) {
153+
break;
154+
}
155+
}
132156
}
157+
}
133158

134-
fclose($namedpipes->stderr);
135-
fclose($namedpipes->stdout);
159+
public function stop(): int
160+
{
161+
if ($this->resource === null) {
162+
throw new Exception('Command not started');
163+
}
164+
fclose($this->namedPipes->stderr);
165+
fclose($this->namedPipes->stdout);
166+
167+
proc_terminate($this->resource);
136168

137169
$exitCode = proc_close($this->resource);
138170
if ($this->exitCode === null) {
139171
$this->exitCode = $exitCode;
140172
}
141173

142174
$this->resource = null;
175+
$this->namedPipes = null;
143176

144-
return (object) [
145-
'stdout' => $stdout,
146-
'stderr' => $stderr,
147-
];
177+
return $exitCode;
148178
}
149179
}

0 commit comments

Comments
 (0)