WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 6 months ago

#19487 new defect (bug)

Remove useless calls to set_time_limit()

Reported by: dllh Owned by: westi
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 1.5
Component: Bootstrap/Load Keywords: has-patch
Focuses: Cc:

Description

Calls to set_time_limit() were introduced in http://core.trac.wordpress.org/changeset/1812 and have remained in core ever since. The call occurs in code that makes network connections and is designed to allow time for the network calls to complete before the script execution stops.

But set_time_limit() won't take network time into account, so it actually will not do what it seems designed to do. From php docs:

The set_time_limit() function and the configuration directive max_execution_time only affect the execution time of the script itself. Any time spent on activity that happens outside the execution of the script such as system calls using system(), stream operations, database queries, etc. is not included when determining the maximum time that the script has been running.

Further, calls to set_time_limit() can cause unexpected results in code that relies on any functions that call set_time_limit(). For example, if some code (in a cron job, say) sets the time limit to 0 (unlimited) because it knows it needs some time complete, then a subsequent call to a function that resets the time limit will halt the long-running execution once the new limit has been reached. Also from php docs:

When called, set_time_limit() restarts the timeout counter from zero. In other words, if the timeout is the default 30 seconds, and 25 seconds into script execution a call such as set_time_limit(20) is made, the script will run for a total of 45 seconds before timing out.

Since the call to set_time_limit() does not here do anything useful, it should be removed.

Attachments (1)

remove_set_time_limit2.patch (846 bytes) - added by dllh 3 years ago.
Remove unnecessary calls to set_time_limit()

Download all attachments as: .zip

Change History (6)

dllh3 years ago

Remove unnecessary calls to set_time_limit()

comment:1 kurtpayne3 years ago

  • Cc kpayne@… added

From the php.net page for set_time_limit (emphasis mine):

Note:

The set_time_limit() function and the configuration directive max_execution_time only affect the execution time of the script itself. Any time spent on activity that happens outside the execution of the script such as system calls using system(), stream operations, database queries, etc. is not included when determining the maximum time that the script has been running. This is not true on Windows where the measured time is real.

The potential harm of resetting the execution time counter should be measured against the value of extending the execution time for Windows.

Is this causing a problem on your site?

A proposed workaround to reset the time limit for your site:

add_filter( 'http_request_timeout', function() {
    set_time_limit(60); // Your time limit here
    return 5;
}

comment:2 kurtpayne3 years ago

Related #19486

Last edited 3 years ago by kurtpayne (previous) (diff)

comment:3 nacin3 years ago

  • Version changed from 3.3 to 1.5

comment:5 nacin6 months ago

  • Component changed from General to Bootstrap/Load
Note: See TracTickets for help on using tickets.