diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 0000000..8e3ed60 --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1 @@ +github: Toflar \ No newline at end of file diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..608cecd --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,75 @@ +name: CI + +on: + pull_request: ~ + schedule: + - cron: 0 13 * * MON + +jobs: + cs: + name: Coding Style + runs-on: ubuntu-latest + steps: + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.3' + coverage: none + + - name: Checkout + uses: actions/checkout@v3 + + - name: Install the dependencies + run: composer update --no-interaction --no-suggest + + - name: Run the CS fixer + run: composer cs-fixer + + tests-linux: + name: 'Linux: PHP ${{ matrix.php }}' + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + php: ['8.1', '8.2', '8.3'] + steps: + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: none + + - name: Checkout + uses: actions/checkout@v3 + + - name: Install the dependencies + run: composer update --no-interaction --no-suggest + + - name: Run the unit tests + run: composer unit-tests + + tests-windows: + name: 'Windows: PHP ${{ matrix.php }}' + runs-on: windows-latest + strategy: + fail-fast: false + matrix: + php: ['8.1', '8.2', '8.3'] + steps: + - name: Set up Cygwin + uses: egor-tensin/setup-cygwin@v4 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: none + + - name: Checkout + uses: actions/checkout@v3 + + - name: Install the dependencies + run: composer update --no-interaction --no-suggest + + - name: Run the unit tests + run: composer unit-tests diff --git a/composer.json b/composer.json index a9cbb7c..4331a25 100644 --- a/composer.json +++ b/composer.json @@ -4,9 +4,9 @@ "type": "library", "require": { "php": "^8.1", - "symfony/process": "^6.0", - "symfony/lock": "^6.0", - "symfony/filesystem": "^6.0" + "symfony/process": "^6.0 || ^7.0", + "symfony/lock": "^6.0 || ^7.0", + "symfony/filesystem": "^6.0 || ^7.0" }, "require-dev": { "phpunit/phpunit": "^10.2", @@ -33,5 +33,8 @@ "allow-plugins": { "terminal42/contao-build-tools": true } + }, + "scripts": { + "unit-tests": "@php vendor/bin/phpunit" } } diff --git a/src/BasicCommand.php b/src/BasicCommand.php index 889103c..bb7c0e7 100644 --- a/src/BasicCommand.php +++ b/src/BasicCommand.php @@ -14,7 +14,6 @@ public function __construct( /** @var \Closure():Process */ private readonly \Closure $createProcess, ) { - } public function getIdentifier(): string diff --git a/src/Provider/PosixProvider.php b/src/Provider/PosixProvider.php new file mode 100644 index 0000000..23180ae --- /dev/null +++ b/src/Provider/PosixProvider.php @@ -0,0 +1,19 @@ +mustRun(); + + return true; + } catch (\Throwable) { + return false; + } + } + + public function isPidRunning(int $pid): bool + { + try { + $process = new Process(['ps', '-p', $pid]); + $process->mustRun(); + + // Check for defunct output. If the process was started within this very process, + // it will still be listed, although it's actually finished. + if (str_contains($process->getOutput(), '')) { + return false; + } + + return true; + } catch (\Throwable) { + return false; + } + } +} diff --git a/src/Provider/WindowsTaskListProvider.php b/src/Provider/WindowsTaskListProvider.php new file mode 100644 index 0000000..aa840c2 --- /dev/null +++ b/src/Provider/WindowsTaskListProvider.php @@ -0,0 +1,39 @@ +mustRun(); + + return true; + } catch (\Throwable) { + return false; + } + } + + public function isPidRunning(int $pid): bool + { + try { + $process = new Process(['tasklist', '/FI', "PID eq $pid"]); + $process->mustRun(); + + // Symfony Process starts Windows processes via cmd.exe + return str_contains($process->getOutput(), 'cmd.exe'); + } catch (\Throwable) { + return false; + } + } +} diff --git a/src/Supervisor.php b/src/Supervisor.php index 1078bfe..0170b98 100644 --- a/src/Supervisor.php +++ b/src/Supervisor.php @@ -8,12 +8,17 @@ use Symfony\Component\Lock\LockFactory; use Symfony\Component\Lock\Store\FlockStore; use Symfony\Component\Process\Process; +use Toflar\CronjobSupervisor\Provider\PosixProvider; +use Toflar\CronjobSupervisor\Provider\ProviderInterface; +use Toflar\CronjobSupervisor\Provider\PsProvider; +use Toflar\CronjobSupervisor\Provider\WindowsTaskListProvider; class Supervisor { private const LOCK_NAME = 'cronjob-supervisor-lock'; private readonly LockFactory $lockFactory; + private readonly Filesystem $filesystem; /** @@ -31,14 +36,66 @@ class Supervisor */ private array $childProcesses = []; - public function __construct(private readonly string $storageDirectory) - { + /** + * @param array $providers + */ + private function __construct( + private readonly string $storageDirectory, + private readonly array $providers, + ) { $this->lockFactory = new LockFactory(new FlockStore($storageDirectory)); $this->filesystem = new Filesystem(); $this->filesystem->mkdir($this->storageDirectory); } + public static function withDefaultProviders(string $storageDirectory): self + { + return new self($storageDirectory, self::getDefaultProviders()); + } + + public static function getDefaultProviders(): array + { + return [ + new WindowsTaskListProvider(), + new PosixProvider(), + new PsProvider(), + ]; + } + + /** + * @param array $providers + */ + public static function withProviders(string $storageDirectory, array $providers): self + { + return new self($storageDirectory, $providers); + } + + /** + * @param array $providers + */ + public static function canSuperviseWithProviders(array $providers): bool + { + foreach ($providers as $provider) { + if ($provider->isSupported()) { + return true; + } + } + + return false; + } + + public function canSupervise(): bool + { + foreach ($this->providers as $provider) { + if ($provider->isSupported()) { + return true; + } + } + + return false; + } + public function withCommand(CommandInterface $command): self { $clone = clone $this; @@ -52,6 +109,10 @@ public function withCommand(CommandInterface $command): self */ public function supervise(\Closure|null $onTick = null): void { + if (!$this->canSupervise()) { + throw new \LogicException('No provider supported, cannot supervise!'); + } + $end = time() + 55; $tick = 1; @@ -69,8 +130,9 @@ public function supervise(\Closure|null $onTick = null): void ++$tick; } - // Okay, we are done supervising. Now we might have child processes that are still running. We have to wait - // for them to finish. Only then we can exit ourselves otherwise we'd kill the children + // Okay, we are done supervising. Now we might have child processes that are + // still running. We have to wait for them to finish. Only then we can exit + // ourselves otherwise we'd kill the children while ($this->hasRunningChildProcesses()) { sleep(5); } @@ -105,7 +167,7 @@ function (): void { // Save state $this->filesystem->dumpFile($this->getStorageFile(), json_encode($this->storage, JSON_THROW_ON_ERROR)); - } + }, ); } @@ -139,8 +201,8 @@ private function padCommand(CommandInterface $command): void if (null !== $process->getPid()) { $this->storage[$command->getIdentifier()][] = $process->getPid(); - // Remember started child processes because we have to remain running in order for those child processes - // not to get killed. + // Remember started child processes because we have to remain running in order + // for those child processes not to get killed. $this->childProcesses[$process->getPid()] = $process; } } @@ -154,8 +216,8 @@ private function getStorageFile(): string private function executeLocked(\Closure $closure): void { - // Library is meant to be used with minutely cronjobs. Thus, the default ttl of 300 is enough and does not need - // to be configurable. + // Library is meant to be used with minutely cronjobs. Thus, the default ttl of + // 300 is enough and does not need to be configurable. $lock = $this->lockFactory->createLock(self::LOCK_NAME); if (!$lock->acquire()) { return; @@ -167,19 +229,12 @@ private function executeLocked(\Closure $closure): void private function isRunningPid(int $pid): bool { - $process = new Process(['ps', '-p', $pid]); - $exitCode = $process->run(); - - if (0 !== $exitCode) { - return false; - } - - // Check for defunct output. If the process was started within this very process, it will still be listed, - // although it's actually finished. - if (str_contains($process->getOutput(), '')) { - return false; + foreach ($this->providers as $provider) { + if ($provider->isSupported()) { + return $provider->isPidRunning($pid); + } } - return true; + return false; } } diff --git a/tests/SupervisorTest.php b/tests/SupervisorTest.php index 1ff4def..549793c 100644 --- a/tests/SupervisorTest.php +++ b/tests/SupervisorTest.php @@ -7,11 +7,24 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Process\PhpExecutableFinder; use Symfony\Component\Process\Process; +use Toflar\CronjobSupervisor\Supervisor; class SupervisorTest extends TestCase { + public function testCanSupervise(): void + { + $supervisor = Supervisor::withProviders(sys_get_temp_dir(), []); + $this->assertFalse($supervisor->canSupervise()); + $this->assertFalse(Supervisor::canSuperviseWithProviders([])); + } + public function testSupervising(): void { + $supervisor = Supervisor::withDefaultProviders(sys_get_temp_dir()); + if (!$supervisor->canSupervise()) { + $this->markTestSkipped('Supervising is not supperted.'); + } + $start = time(); $php = (new PhpExecutableFinder())->find(); @@ -23,7 +36,8 @@ public function testSupervising(): void // Simulate concurrent cron (this should NOT cause additional workers to be started!) $processes[] = $this->simulateRunner($php); - // Simulate yet another concurrent cron (this should NOT cause additional workers to be started!) + // Simulate yet another concurrent cron (this should NOT cause additional workers + // to be started!) $processes[] = $this->simulateRunner($php); while (true) { @@ -42,8 +56,8 @@ public function testSupervising(): void sleep(5); } - // The runner.php has a process that runs 100 seconds, so our supervisor must run at least 100 seconds, otherwise - // it would've killed the child process + // The runner.php has a process that runs 100 seconds, so our supervisor must run + // at least 100 seconds, otherwise it would've killed the child process $this->assertGreaterThanOrEqual(100, time() - $start); } @@ -53,7 +67,7 @@ private function simulateRunner(string $php): Process $p->start( function (): void { $this->assertLessThanOrEqual(6, $this->countSleepProcesses()); - } + }, ); return $p; diff --git a/var/runner.php b/var/runner.php index d1a2959..8ec2fe8 100644 --- a/var/runner.php +++ b/var/runner.php @@ -6,7 +6,7 @@ use Toflar\CronjobSupervisor\BasicCommand; use Toflar\CronjobSupervisor\Supervisor; -(new Supervisor(__DIR__ . '/storage')) +(Supervisor::withDefaultProviders(__DIR__ . '/storage')) ->withCommand(new BasicCommand('sleep 10', 2, function () { return new Process(['sleep', '10']); }))