wpengine / phpcompat

WordPress Plugin: PHP Compatibility Checker
https://wordpress.org/plugins/php-compatibility-checker/
118 stars 38 forks source link

PHP Fatal error: Maximum execution time #32

Open Pross opened 8 years ago

Pross commented 8 years ago

During scans the scanner hits PHPs max execution time and carries on regardless, so results may be unreliable. I tried messing with registering a shutdown function but thats a bit 'hacky'. Setting the max execution via ini_set is also an option but a lot of hosts disable that function. Maybe add a notice to the plugin page somewhere if the users 'max_execution_time' is < 90s

Heres the debug I got in my log

[08-Jul-2016 20:26:14 UTC] WPE PHP Compatibility: startScan: 1 [08-Jul-2016 20:26:14 UTC] WPE PHP Compatibility: lock: 1 [08-Jul-2016 20:26:14 UTC] WPE PHP Compatibility: scan status: [08-Jul-2016 20:26:14 UTC] WPE PHP Compatibility: Generating directory list. [08-Jul-2016 20:26:14 UTC] WPE PHP Compatibility: 3 plugins left to process. [08-Jul-2016 20:26:14 UTC] WPE PHP Compatibility: Processing: PageLines Framework [08-Jul-2016 20:26:14 UTC] WPE PHP Compatibility: Attempted scan count: 1 [08-Jul-2016 20:26:18 UTC] WPE PHP Compatibility: Processing: PageLines Section Magazine [08-Jul-2016 20:26:18 UTC] WPE PHP Compatibility: Attempted scan count: 1 [08-Jul-2016 20:26:19 UTC] WPE PHP Compatibility: Processing: PageLines Platform 5 [08-Jul-2016 20:26:19 UTC] WPE PHP Compatibility: Attempted scan count: 1 [08-Jul-2016 20:27:09 UTC] PHP Fatal error: Maximum execution time of 55 seconds exceeded in /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php on line 2308 [08-Jul-2016 20:27:09 UTC] PHP Stack trace: [08-Jul-2016 20:27:09 UTC] PHP 1. {main}() /Users/pross/Repos/WordPress/wp-admin/admin-ajax.php:0 [08-Jul-2016 20:27:09 UTC] PHP 2. do_action() /Users/pross/Repos/WordPress/wp-admin/admin-ajax.php:91 [08-Jul-2016 20:27:09 UTC] PHP 3. call_user_func_array:{/Users/pross/Repos/WordPress/wp-includes/plugin.php:524}() /Users/pross/Repos/WordPress/wp-includes/plugin.php:524 [08-Jul-2016 20:27:09 UTC] PHP 4. WPEngine_PHPCompat->start_test() /Users/pross/Repos/WordPress/wp-includes/plugin.php:524 [08-Jul-2016 20:27:09 UTC] PHP 5. WPEPHPCompat->start_test() /Users/pross/Repos/phpcompat/wpengine-phpcompat.php:90 [08-Jul-2016 20:27:09 UTC] PHP 6. WPEPHPCompat->process_file() /Users/pross/Repos/phpcompat/src/wpephpcompat.php:171 [08-Jul-2016 20:27:09 UTC] PHP 7. PHP_CodeSniffer_CLI->process() /Users/pross/Repos/phpcompat/src/wpephpcompat.php:220 [08-Jul-2016 20:27:09 UTC] PHP 8. PHP_CodeSniffer->processFiles() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php:912 [08-Jul-2016 20:27:09 UTC] PHP 9. PHP_CodeSniffer->processFile() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer.php:629 [08-Jul-2016 20:27:09 UTC] PHP 10. PHP_CodeSniffer->_processFile() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer.php:1739 [08-Jul-2016 20:27:09 UTC] PHP 11. PHP_CodeSniffer_File->start() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer.php:1861 [08-Jul-2016 20:27:09 UTC] PHP 12. PHP_CodeSniffer_File->_parse() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:474 [08-Jul-2016 20:27:09 UTC] PHP 13. PHP_CodeSniffer_File::tokenizeString() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:695 [08-Jul-2016 20:27:09 UTC] PHP 14. PHP_CodeSniffer_File::_createScopeMap() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:1448 [08-Jul-2016 20:27:09 UTC] PHP 15. PHP_CodeSniffer_File::_recurseScopeMap() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:1863 [08-Jul-2016 20:27:09 UTC] PHP 16. PHP_CodeSniffer_File::_recurseScopeMap() /Users/pross/Repos/phpcompat/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:2232 [08-Jul-2016 20:27:16 UTC] WPE PHP Compatibility: startScan: [08-Jul-2016 20:27:16 UTC] WPE PHP Compatibility: lock: [08-Jul-2016 20:27:16 UTC] WPE PHP Compatibility: scan status: 1 [08-Jul-2016 20:27:16 UTC] WPE PHP Compatibility: 1 plugins left to process. [08-Jul-2016 20:27:16 UTC] WPE PHP Compatibility: Processing: PageLines Platform 5 [08-Jul-2016 20:27:16 UTC] WPE PHP Compatibility: Attempted scan count: 2 [08-Jul-2016 20:28:08 UTC] WPE PHP Compatibility: Scan finished.

octalmage commented 8 years ago

The scan runs a cron every minute and sets the max execution limit to 55. This way the results are consistent across hosts. If the process gets killed before 55 seconds, it picks back up after a minute. If there is no limit, it gets killed after 55 seconds and starts back up. The problem is when there is no limit, we are limiting how long the process can run and how much can be scanned.

octalmage commented 8 years ago

TLDR: this is expected, but it could be improved.

The first improvement would be to keep track of the run time and kill the process on our own instead of relying on the max execution limit. This would get rid of the fatal errors.

Another improvement would be to kick off the next process when we kill the current one. This way we don't have to rely on the cron.

Pross commented 8 years ago

Cron relies on the site being live and there being some activity for it to trigger though?

I was running the plugin locally so cron wouldnt be activated.

octalmage commented 8 years ago

Correct, this is another one of the reasons I would like to move away from the cron. As mentioned above having the scan "fork" and kick off the next batch would help with this, but that comes with it's own issues.

octalmage commented 8 years ago

I didn't have any issues locally though. I assumed this was due to the Ajax call that hits every 5 seconds, but looking at the core code this might not solve that issue.

octalmage commented 8 years ago

I could add a second Ajax call to domain.com/wp-cron.php?doing_wp_cron to make sure the cron gets fired which might be a good idea for now.

Pross commented 8 years ago

Might be a good idea, feel free to send a patch my way to test

octalmage commented 8 years ago

Awesome thanks!

octalmage commented 8 years ago

This issue conflicts with #19, which I was planning on working on this week. If you leave the page, the JavaScript to trigger the cron won't work. Another solution would be to allow users to disable or configure the timeout. If the plugin can process all plugins in one pass, a cron isn't needed. Would this work for you?