Skip to content

Commit 3a28242

Browse files
asgrimasgrim
authored
Merge pull request #426 from asgrim/remove-timeouts-on-processes-except-sudo
Remove timeouts on all process invocations except those potentially using sudo
2 parents 2b573c6 + c82fe3c commit 3a28242

File tree

12 files changed

+26
-33
lines changed

12 files changed

+26
-33
lines changed

‎src/Building/UnixBuild.php‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@
2626
/** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
2727
final class UnixBuild implements Build
2828
{
29-
private const PHPIZE_TIMEOUT_SECS = 60; // 1 minute
30-
private const CONFIGURE_TIMEOUT_SECS = 120; // 2 minutes
31-
private const MAKE_TIMEOUT_SECS = null; // unlimited
32-
3329
/** {@inheritDoc} */
3430
public function __invoke(
3531
DownloadedPackage $downloadedPackage,
@@ -137,8 +133,7 @@ private function phpize(
137133
Process::run(
138134
$phpizeCommand,
139135
$downloadedPackage->extractedSourcePath,
140-
self::PHPIZE_TIMEOUT_SECS,
141-
$outputCallback,
136+
outputCallback: $outputCallback,
142137
);
143138
}
144139

@@ -162,8 +157,7 @@ private function configure(
162157
Process::run(
163158
$configureCommand,
164159
$downloadedPackage->extractedSourcePath,
165-
self::CONFIGURE_TIMEOUT_SECS,
166-
$outputCallback,
160+
outputCallback: $outputCallback,
167161
);
168162
}
169163

@@ -195,8 +189,7 @@ private function make(
195189
Process::run(
196190
$makeCommand,
197191
$downloadedPackage->extractedSourcePath,
198-
self::MAKE_TIMEOUT_SECS,
199-
$outputCallback,
192+
outputCallback: $outputCallback,
200193
);
201194
}
202195

@@ -231,8 +224,7 @@ private function cleanup(
231224
Process::run(
232225
$phpizeCleanCommand,
233226
$downloadedPackage->extractedSourcePath,
234-
self::PHPIZE_TIMEOUT_SECS,
235-
$outputCallback,
227+
outputCallback: $outputCallback,
236228
);
237229

238230
$io->write('<info>Build files cleaned up.</info>');

‎src/ComposerIntegration/PhpBinaryPathBasedPlatformRepository.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private function packageForExtension(string $name, string $prettyVersion): Compl
124124
private function detectLibraryWithPkgConfig(string $alias, string $library): void
125125
{
126126
try {
127-
$pkgConfigResult = Process::run(['pkg-config', '--print-provides', '--print-errors', $library], timeout: 30);
127+
$pkgConfigResult = Process::run(['pkg-config', '--print-provides', '--print-errors', $library]);
128128
} catch (ProcessFailedException) {
129129
return;
130130
}

‎src/File/SudoCreate.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public static function file(string $filename): void
5151
}
5252

5353
try {
54-
Process::run([Sudo::find(), 'touch', $filename]);
54+
Process::run([Sudo::find(), 'touch', $filename], timeout: Process::SHORT_TIMEOUT);
5555
} catch (ProcessFailedException $processFailedException) {
5656
throw FailedToCreateFile::fromSudoTouchProcessFailed($filename, $processFailedException);
5757
}

‎src/File/SudoFilePut.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ private static function writeWithSudo(string $filename, string $content): void
6666
self::copyOwnership($filename, $tempFilename);
6767
}
6868

69-
Process::run([Sudo::find(), 'mv', $tempFilename, $filename]);
69+
Process::run([Sudo::find(), 'mv', $tempFilename, $filename], timeout: Process::SHORT_TIMEOUT);
7070
}
7171

7272
/**
@@ -77,18 +77,18 @@ private static function copyOwnership(string $sourceFile, string $targetFile): v
7777
{
7878
try {
7979
// GNU chmod supports `--reference`, so try this first
80-
Process::run([Sudo::find(), 'chmod', '--reference=' . $sourceFile, $targetFile]);
80+
Process::run([Sudo::find(), 'chmod', '--reference=' . $sourceFile, $targetFile], timeout: Process::SHORT_TIMEOUT);
8181

8282
return;
8383
} catch (ProcessFailedException) {
8484
// Fall back to using `stat` to determine uid/gid
8585
try {
8686
// Try using GNU stat (-c) first
87-
$userAndGroup = Process::run(['stat', '-c', '%u:%g', $sourceFile], timeout: 2);
87+
$userAndGroup = Process::run(['stat', '-c', '%u:%g', $sourceFile]);
8888
} catch (ProcessFailedException) {
8989
try {
9090
// Fall back to using OSX stat (-f)
91-
$userAndGroup = Process::run(['stat', '-f', '%u:%g', $sourceFile], timeout: 2);
91+
$userAndGroup = Process::run(['stat', '-f', '%u:%g', $sourceFile]);
9292
} catch (ProcessFailedException) {
9393
return;
9494
}
@@ -98,7 +98,7 @@ private static function copyOwnership(string $sourceFile, string $targetFile): v
9898
return;
9999
}
100100

101-
Process::run([Sudo::find(), 'chown', $userAndGroup, $targetFile]);
101+
Process::run([Sudo::find(), 'chown', $userAndGroup, $targetFile], timeout: Process::SHORT_TIMEOUT);
102102
}
103103
}
104104
}

‎src/File/SudoUnlink.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public static function singleFile(string $filename): void
4747
}
4848

4949
try {
50-
Process::run([Sudo::find(), 'rm', $filename]);
50+
Process::run([Sudo::find(), 'rm', $filename], timeout: Process::SHORT_TIMEOUT);
5151
} catch (ProcessFailedException $processFailedException) {
5252
throw FailedToUnlinkFile::fromSudoRmProcessFailed($filename, $processFailedException);
5353
}

‎src/Installing/Ini/OndrejPhpenmod.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ static function () use ($needSudo, $phpenmodPath, $targetPlatform, $downloadedPa
166166
array_unshift($processArgs, Sudo::find());
167167
}
168168

169-
Process::run($processArgs);
169+
Process::run($processArgs, timeout: Process::SHORT_TIMEOUT);
170170

171171
return true;
172172
} catch (ProcessFailedException $processFailedException) {

‎src/Installing/InstallForPhpProject/InstallSelectedPackage.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ static function (string $value, string $key) use (&$process): void {
6060
Process::run(
6161
$process,
6262
getcwd(),
63-
null,
64-
static function (string $outOrErr, string $message) use ($io): void {
63+
outputCallback: static function (string $outOrErr, string $message) use ($io): void {
6564
if ($outOrErr === \Symfony\Component\Process\Process::ERR) {
6665
$io->writeError(' > ' . $message);
6766

‎src/Installing/UninstallUsingUnlink.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public function __invoke(Package $package): BinaryFile
4646

4747
// If the target directory isn't writable, or a .so file already exists and isn't writable, try to use sudo
4848
if (file_exists($expectedBinaryFile->filePath) && ! is_writable($expectedBinaryFile->filePath) && Sudo::exists()) {
49-
Process::run([Sudo::find(), 'rm', $expectedBinaryFile->filePath]);
49+
Process::run([Sudo::find(), 'rm', $expectedBinaryFile->filePath], timeout: Process::SHORT_TIMEOUT);
5050

5151
// Removal worked, bail out
5252
if (! file_exists($expectedBinaryFile->filePath)) {

‎src/SelfManage/Verify/GithubCliAttestationVerification.php‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,8 @@
1919
/** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
2020
final class GithubCliAttestationVerification implements VerifyPiePhar
2121
{
22-
private const GH_CLI_NAME = 'gh';
23-
private const GH_ATTESTATION_COMMAND = 'attestation';
24-
private const GH_VERIFICATION_TIMEOUT = 30;
22+
private const GH_CLI_NAME = 'gh';
23+
private const GH_ATTESTATION_COMMAND = 'attestation';
2524

2625
public function __construct(private readonly ExecutableFinder $executableFinder)
2726
{
@@ -37,7 +36,7 @@ public function verify(ReleaseMetadata $releaseMetadata, BinaryFile $pharFilenam
3736

3837
// Try to use `gh attestation --help` to ensure it is not an old `gh` cli version
3938
try {
40-
Process::run([$gh, self::GH_ATTESTATION_COMMAND, '--help'], null, self::GH_VERIFICATION_TIMEOUT);
39+
Process::run([$gh, self::GH_ATTESTATION_COMMAND, '--help']);
4140
} catch (ProcessFailedException $attestationCommandCheck) {
4241
if (str_starts_with($attestationCommandCheck->getProcess()->getErrorOutput(), sprintf('unknown command "%s" for "%s"', self::GH_ATTESTATION_COMMAND, self::GH_CLI_NAME))) {
4342
throw GithubCliNotAvailable::withMissingAttestationCommand(self::GH_CLI_NAME);
@@ -60,7 +59,7 @@ public function verify(ReleaseMetadata $releaseMetadata, BinaryFile $pharFilenam
6059
);
6160

6261
try {
63-
Process::run($verificationCommand, null, self::GH_VERIFICATION_TIMEOUT);
62+
Process::run($verificationCommand);
6463
} catch (ProcessFailedException $processFailedException) {
6564
throw FailedToVerifyRelease::fromGhCliFailure($releaseMetadata, $processFailedException);
6665
}

‎src/Util/Process.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
/** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
1313
final class Process
1414
{
15+
public const NO_TIMEOUT = null;
16+
public const SHORT_TIMEOUT = 10;
17+
1518
private function __construct()
1619
{
1720
}
@@ -33,7 +36,7 @@ private function __construct()
3336
public static function run(
3437
array $command,
3538
string|null $workingDirectory = null,
36-
int|null $timeout = 5,
39+
int|null $timeout = self::NO_TIMEOUT,
3740
callable|null $outputCallback = null,
3841
): string {
3942
return trim((new SymfonyProcess($command, $workingDirectory, timeout: $timeout))

0 commit comments

Comments
 (0)