php / php-src

The PHP Interpreter
https://www.php.net
Other
38.09k stars 7.74k forks source link

max_execution_time / set_time_limit is inconsistent across OSs and unhelpful on non-windows OSs #8567

Closed SpencerMalone closed 1 year ago

SpencerMalone commented 2 years ago

Description

The following code:

<?php

sleep(6000);
echo("hi");

Resulted in this output:

hi

But I expected this output instead:

Maximum execution time of <x> exceeded

While this is a documented limitation, I would like to argue that this should be reimplemented to match the windows implementation where it tracks the real time of the script.

  1. Max execution time being based on PHP's CPU time is unintuitive. When you set max execution time, nobody expects it to be PHP CPU time, making this an easy to trap where you think you've set yourself up to be safe against long running requests, and instead are still exposed to this failure mode.
  2. This behavior should be OS consistent. Having it inconsistent adds to operational confusion when working with PHP.
  3. If the expectation is that the SAPI be the one to handle "real" max execution time, then one of the oldest simplest SAPIs (apache's mod_php) does not have a way to handle this, and PHP CLI applications will be left in the dust, as they have no comparable way to manage max execution time.
  4. Max execution time being based on CPU time is not useful. If a request is running for 5+ minutes, I don't care if it's sleeping, stuck in a bad recursive loop, waiting on a bad mysql query, or stuck on a curl that won't resolve, I just need it to end and free up resources. Without this, I've seen requests that have lifespans of days or weeks because they've gotten stuck in novel places without a timeout.

PHP Version

7.4

Operating System

Not-windows

SpencerMalone commented 2 years ago

I wasn't 100% sure if this is a bug or a feature request, to me it feels like a documented bug, so I dropped it in that category. If it needs to be swapped, I'm totally fine with that.

damianwadley commented 2 years ago

I wasn't 100% sure if this is a bug or a feature request, to me it feels like a documented bug, so I dropped it in that category. If it needs to be swapped, I'm totally fine with that.

I'd call it more of a feature request: it's not like the behaviors are random decisions but conscious thought was put into how it works, and changing that will (probably) require some amount of effort and planning. It would also be a significant change that should therefore wait until PHP 9.0.

cmb69 commented 2 years ago

See https://github.com/php/php-src/pull/6504.

github-actions[bot] commented 2 years ago

There has not been any recent activity in this feature request. It will automatically be closed in 14 days if no further action is taken. Please see https://github.com/probot/stale#is-closing-stale-issues-really-a-good-idea to understand why we auto-close stale feature requests.

SpencerMalone commented 2 years ago

That PR/RFC 110% solves my needs. I also poked around with a simple homebrewed extension that also utilizes itimers with success. I also appreciate the context from https://github.com/php/php-src/pull/6504#issuecomment-870986910 showing that the design considerations that originally drove this are no longer applicable. For those that want it, the RFC is also visible @ https://wiki.php.net/rfc/max_execution_wall_time

cmb69 commented 2 years ago

@kocsismate, are you planning to pursue https://wiki.php.net/rfc/max_execution_wall_time sometime? :)

kocsismate commented 2 years ago

Are you planning to pursue https://wiki.php.net/rfc/max_execution_wall_time sometime? :)

Yes, definitely! I hope that we'll have time to settle on a solution for PHP 8.3 :)

github-actions[bot] commented 1 year ago

There has not been any recent activity in this feature request. It will automatically be closed in 14 days if no further action is taken. Please see https://github.com/probot/stale#is-closing-stale-issues-really-a-good-idea to understand why we auto-close stale feature requests.