Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update worker.md to prevent infinite loop #455

Merged
merged 3 commits into from
Jan 8, 2024
Merged

Update worker.md to prevent infinite loop #455

merged 3 commits into from
Jan 8, 2024

Conversation

alexandreelise
Copy link
Contributor

Quick fix typo <= rather than >= which prevents infinite loop and get out of the loop when MAX_REQUESTS is reached.

Quick fix typo `<=` rather than `>=` which prevents infinite loop and get out of the loop when MAX_REQUESTS is reached.
Fix impossible case where if MAX_REQUESTS is not defined the rest of the condition is not evaluated as it is an && operator as mentioned by #453 (comment)
docs/worker.md Outdated
@@ -80,7 +80,7 @@ do {

// Call the garbage collector to reduce the chances of it being triggered in the middle of a page generation
gc_collect_cycles();
} while ($running && !(isset($_SERVER['MAX_REQUESTS']) && ++$nbRequests >= $_SERVER['MAX_REQUESTS']));
} while ($running && isset($_SERVER['MAX_REQUESTS']) && (++$nbRequests <= $_SERVER['MAX_REQUESTS']));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth considering a for-loop instead of a while loop:

for($nbRequests = 0, $running = true; $nbRequests < $_SERVER['MAX_REQUESTS'] && $running; $nbRequests++) {
 // do request stuff
}

as well as moving the $handler out of the loop so that a new closure isn't created on every request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe with pre-increment because it's sligthly faster. when converted to OPCODES @withinboredom

Change to for loop and remove handler from the loop based on the feedback #455 (comment)
@alexandreelise
Copy link
Contributor Author

Binary releases ci build step might be failing because of this

#453 (reply in thread)

@dunglas dunglas merged commit f19c153 into dunglas:main Jan 8, 2024
26 of 28 checks passed
@dunglas
Copy link
Owner

dunglas commented Jan 8, 2024

Thanks!

@alexandreelise alexandreelise deleted the patch-1 branch May 4, 2024 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants