Opened 10 years ago
Closed 9 years ago
#31625 closed defect (bug) (fixed)
Function register_uninstall_hook doesn't have a method to remove the outdated data
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | 4.1.1 |
Component: | Plugins | Keywords: | has-patch needs-testing |
Focuses: | performance | Cc: |
Description
Function register_uninstall_hook doesn't have a method to remove the outdated data. Here's a problem that I faced recently:
Subheading plugin used register_uninstall_hook to register its uninstaller in the old versions. But in new version register_uninstall_hook call was removed and uninstall.php was added.
And the plugin file was renamed from index.php to subheading.php.
As a result this is what I get in a serialized array:
["subheading/index.php"]=> array(2) { [0]=> object(__PHP_Incomplete_Class)#242 (5) {
The digit after __PHP_Incomplete_Class
is always different, so on every update_option call Wordpress lauches MySQL
"UPDATE `wp_options` SET `option_value` = 'a:8:{i:0;b:0;s:33 ".
But because writing to a field locks a table row, this causes the situation when server in fact can process only one visitor at a time.
Attachments (2)
Change History (12)
#2
@
9 years ago
- Milestone changed from Awaiting Review to 4.4
It looks like register_uninstall_hook()
was used incorrect here in the first place, register_uninstall_hook()
is designed to be given a function callback (or a static class variable), NOT a array/object instance like was provided here.
We've blocked those being used since [16339] so the plugin in question would've had to be run over 5 years ago (Okay, so it could be more recent if they'd not updated their site).
There's three options here:
- wontfix
- add a pre option save filter to make sure it only ever contains the right contents
- add a once-off upgrade routine to check to see if it contains any non-string/static callbacks
I see nothing wrong with doing #3 here, so if someone wants to whip up a patch before I do..
Do we have any other arrays in options which store similar data?
#4
@
9 years ago
- Keywords punt added
- Milestone changed from 4.4 to Future Release
Punting to Future Release in lieu of a patch. See comment:2 for suggested approaches.
#6
@
9 years ago
- Keywords has-patch needs-testing added; needs-patch removed
Patch added with upgrade routine. I incremented the current db version to an arbitrary 36999 as I wasn't sure how it would be calculated. I have also assumed this would be for 4.6.
#7
@
9 years ago
- Milestone changed from Future Release to 4.6
- Owner set to dd32
- Status changed from new to accepted
Other than not needing to check $update
(update_option()
will check if the value has changed) 31625.diff seems good to me.
You don't need to create a new patch for that though, can strip that out during commit easily enough (feel free to upload a new one if you'd like)
I incremented the current db version to an arbitrary 36999 as I wasn't sure how it would be calculated.
We set it to the SVN Commit number of the commit we're making when the patch is committed, so an arbitrary number is good :) although I usually just use +1 (and then +1 again for the next test iteration.. etc)
@dd32: Would you mind taking a gander at this?