#49291 closed defect (bug) (fixed)
Code issues with set_time_limit
Reported by: | madpeter | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 6.8 | Priority: | normal |
Severity: | normal | Version: | 5.3.2 |
Component: | Revisions | Keywords: | needs-patch good-first-bug |
Focuses: | administration, performance | Cc: |
Description (last modified by )
file: wp-admin\includes\ajax-actions.php
function: wp_ajax_get_revision_diffs
line: 3425
Issue: no time limit set
Suggestion: set value to 120
file: wp-includes\class-pop3.php
function: __construct
line: 63
Issue: timeout value is not tested and can be set to 9999999 or a negitive value that acts the same as 0
Suggestion: Force a range for the value (1 to 120)
Attachments (1)
Change History (12)
#1
@
5 years ago
- Component changed from Administration to Revisions
- Description modified (diff)
- Focuses administration added
- Severity changed from major to normal
#2
@
5 years ago
- Keywords reporter-feedback added
The set_time_limit( 0 ) instance in wp_ajax_get_revision_diffs() was introduced in [24707] / #24757. Could you clarify why the value should be changed to 120?
if a badly coded plugin calls wp_ajax_get_revision_diffs but does not call set_time_limit itself and then it hangs due to a bug it can hold php active and not free up memory or worse use a while loop to hog cpu time.
but having a known limit for all set_time_limit calls it can help highlight issues with poorly coded plugins as it will create php time out warnings.
class-pop3.php
I will need to open a new ticket as wordpresses copy of phpmailer to far out of date.
#4
@
2 months ago
- Resolution set to fixed
- Status changed from new to closed
Fixed in [24707]
Closing ticket
#5
@
3 weeks ago
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for another look.
@pbearne was [24707] the changeset you meant to share? That commit was 11 years ago but this ticket was only opened 5 years ago.
#7
@
3 weeks ago
- Keywords needs-patch good-first-bug added
- Milestone changed from Awaiting Review to 6.8
This appears to be the only instance of set_time_limit( 0 )
in core, i.e. removing the timeout limit completely.
Other instances have a reasonable limit instead, e.g. set_time_limit( 5 * MINUTE_IN_SECONDS )
.
Looking at #24757 again, I think we could do the same here to avoid hanging processes.
This ticket was mentioned in PR #7638 on WordPress/wordpress-develop by @lovesoni.
3 weeks ago
#8
- Keywords has-patch added; needs-patch removed
Code issues with set_time_limit for ticket: 49291
Trac ticket: https://core.trac.wordpress.org/ticket/49291
I have set limit for timeout to 120 for smooth workflow of ajax and POP3 email
#10
@
3 weeks ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from reopened to closed
In 59288:
@SergeyBiryukov commented on PR #7638:
3 weeks ago
#11
Thanks for the PR! Merged in r59288.
Hi there, welcome to WordPress Trac! Thanks for the report.
set_time_limit( 0 )
instance inwp_ajax_get_revision_diffs()
was introduced in [24707] / #24757. Could you clarify why the value should be changed to 120?class-pop3.php
is a part of the PHPMailer external library, any changes to its files should be submitted upstream: https://github.com/PHPMailer/PHPMailer.