[wp-trac] [WordPress Trac] #60457: Plugin Dependencies: Running update_option within wp-settings can be catastrophic for a high traffic site

WordPress Trac noreply at wordpress.org
Wed Feb 7 12:44:30 UTC 2024


#60457: Plugin Dependencies: Running update_option within wp-settings can be
catastrophic for a high traffic site
--------------------------+--------------------------
 Reporter:  dd32          |       Owner:  costdev
     Type:  defect (bug)  |      Status:  assigned
 Priority:  normal        |   Milestone:  6.5
Component:  Plugins       |     Version:  trunk
 Severity:  normal        |  Resolution:
 Keywords:  dev-feedback  |     Focuses:  performance
--------------------------+--------------------------

Comment (by costdev):

 Following up on this, there are several areas I believe we can improve on.

 **1. Unnecessary logic in the `wp_get_active_and_valid_plugins()` loop in
 `wp-settings.php`**

 - Proposal: Remove the part of the dependencies check in the
 `wp_get_active_and_valid_plugins()` loop that deals with plugins that
 require other plugins.

 `WP_Plugin_Dependencies::initialize()` runs during bootstrap, before the
 `wp_get_active_and_valid_plugins()` loop. This means that
 `WP_Plugin_Dependencies::deactivate_dependents_with_unmet_dependencies()`
 will have already deactivated said plugins if necessary, and therefore the
 `wp_get_active_and_valid_plugins()` loop won't deal with plugins that
 require other plugins. There's no need to have that part of the logic
 included in the loop and therefore running twice.

 -----

 **2. Unnecessary data in the `plugin_data` option**

 - Proposal: Reduce the contents of the `plugin_data` option to just the
 needed headers.

 `WP_Plugin_Dependencies` only uses the `Name`, and `RequiresPlugins`
 headers. We don't need to store all headers for the Plugin Dependencies
 feature. We stored all the headers so that they'd be available to Core
 without looking to the filesystem each time, but the other headers aren't
 being used at this time.

 However, `Requires`, `RequiresPHP` would also be necessary for checking
 WordPress and PHP requirements. They're just not used directly by the
 `WP_Plugin_Dependencies` class, which is focused on plugins that require
 other plugins.

 -----

 **3. Over-cautious updating of the `plugin_data` option**

 - Proposal: Remove the `update_option()` calls in the
 `wp_get_active_and_valid_plugins()` loop

 In the `wp_get_active_and_valid_plugins()`, any plugins that aren't
 included in the `plugin_data` are added. This was an over-cautious attempt
 to avoid plugins going unnoticed, but is unlikely to be a common or even
 occasional issue.

 -----

 **4. Cleanup operation running too often**

 - Proposal: Guard the cleanup operation inside
 `wp_get_active_and_valid_plugins()` to only run on the admin's
 `plugins.php` page.
 - Alternative Proposal 1: Schedule the cleanup in cron. This isn't always
 enabled on WordPress sites, so it would produce an inconsistent result
 across sites.
 - Alternative Proposal 2: Don't clean up. This will by definition result
 in unnecessary data in the option though.

 In `wp_get_active_and_valid_plugins()`, invalid plugins are removed from
 the `plugin_data` option. This is part of a cleanup operation, and while
 useful for reducing the data stored in the database, isn't necessary when
 loading any page. We could just do this when the `plugins.php` (Plugins >
 Installed Plugins) page is visited, or in cron, or not clean up at all.

 -----

 @afragen @azaozz @dd32 Let me know thoughts on each of the above.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/60457#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list