Skip to content

Commit

Permalink
Merge pull request #10 from bhushan/feat/process-fork
Browse files Browse the repository at this point in the history
feat: create child process for calling guzzle http request
  • Loading branch information
JustSteveKing authored Aug 24, 2023
2 parents 31dcbf0 + 6d7ad6d commit 9b4ea33
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 9 deletions.
59 changes: 50 additions & 9 deletions src/Treblle.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
use Treblle\Model\Data;
use Treblle\Model\Error;

use function Safe\getmypid;

/**
* Create a FREE Treblle account => https://treblle.com/register.
*/
Expand Down Expand Up @@ -130,6 +132,8 @@ private function buildPayload(): array

/**
* Process the log when PHP is finished processing.
*
* @throws \Throwable
*/
public function onShutdown(): void
{
Expand All @@ -142,11 +146,44 @@ public function onShutdown(): void
}

/** @todo come up with some kind of fallback to be sent if we cannot convert array to json */
$payload = [];
$payload = '{}';
}

if (!\function_exists('pcntl_fork') || (\defined('ARE_TESTS_RUNNING') && ARE_TESTS_RUNNING)) {
$this->collectData($payload);

return;
}

$pid = pcntl_fork();

if ($this->isUnableToForkProcess($pid)) {
$this->collectData($payload);

return;

This comment has been minimized.

Copy link
@withinboredom

withinboredom Aug 27, 2023

Note that the parent process MUST wait on the PID in order to reap the process -- otherwise, zombie processes will remain and eventually crash the server. Pretty sure, anyway. However I suggest doing something like this:

register_shutdown_function(static function() use ($pid) { 
  fastcgi_finish_request(); // todo: verify function exists
  pcntl_waitpid($pid)
});

Note the static function() so the class can still be destructed if required by the GC. This ensures the current request is closed, then the process waits for the fork to complete.

This comment has been minimized.

Copy link
@JustSteveKing

JustSteveKing Sep 4, 2023

Author Contributor

Thanks for this @withinboredom - I am actually working on a replacement package for the treblle-php package at the moment, which will have a more in-depth integration with the pcntl extension.

Would love your thoughts on it while we build this out if you are open to beta-testing?

Also, I apologise for the delay in replying I was on vacation last week.

}

if ($this->isChildProcess($pid)) {
$this->collectData($payload);
$this->killProcessWithId((int) getmypid());
}
}

public function getBaseUrl(): string
{
$urls = [
'https://rocknrolla.treblle.com',
'https://punisher.treblle.com',
'https://sicario.treblle.com',
];

return $urls[array_rand($urls)];
}

private function collectData(string $payload): void
{
try {
$response = $this->guzzle->request(
$this->guzzle->request(
'POST',
$this->getBaseUrl(),
[
Expand All @@ -168,14 +205,18 @@ public function onShutdown(): void
}
}

public function getBaseUrl(): string
private function isChildProcess(int $pid): bool
{
$urls = [
'https://rocknrolla.treblle.com',
'https://punisher.treblle.com',
'https://sicario.treblle.com',
];
return $pid === 0;
}

return $urls[array_rand($urls)];
private function isUnableToForkProcess(int $pid): bool
{
return $pid === -1;
}

private function killProcessWithId(int $pid): void
{
mb_strtoupper(mb_substr(PHP_OS, 0, 3)) === 'WIN' ? exec("taskkill /F /T /PID {$pid}") : exec("kill -9 {$pid}");
}
}
1 change: 1 addition & 0 deletions tools/phpunit/config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
>
<php>
<ini name="error_reporting" value="-1"/>
<const name="ARE_TESTS_RUNNING" value="true"/>
</php>

<testsuites>
Expand Down

0 comments on commit 9b4ea33

Please sign in to comment.