Make WordPress Core

Opened 12 years ago

Closed 11 years ago

#20950 closed defect (bug) (fixed)

wpmu_delete_blog leaves zombie cache objects

Reported by: mohanjith's profile mohanjith Owned by: wonderboymusic's profile wonderboymusic
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description

For example cached objects with keys like current_blog_{$domain}, current_blog_{$domain}, get_id_from_blogname_{$name} will be left to haunt the network even after the blog is deleted.

Symptoms will be not being allowed to create blogs with the same name after it's deleted, blog settings of old blogs creeping into newly created blogs.

Attachments (5)

missing-cache-delete.diff (813 bytes) - added by wonderboymusic 12 years ago.
missing-cache-delete.2.diff (991 bytes) - added by wonderboymusic 12 years ago.
20950.diff (2.2 KB) - added by ryan 11 years ago.
Rename $name to $slug for some clarity.
20950-ut.diff (1.9 KB) - added by ryan 11 years ago.
Unit tests and blog factory fix
20950-ut.2.diff (2.7 KB) - added by ryan 11 years ago.
More cache tests

Download all attachments as: .zip

Change History (20)

#1 @wonderboymusic
12 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.5

I feel dirty after debugging this. Here's the flow:

1) call wpmu_delete_blog()

2) It calls update_blog_status( $blog_id, 'deleted', 1 );

3) ...which calls refresh_blog_details($blog_id);

4) Which does:

wp_cache_delete( $blog_id , 'blog-details' );
wp_cache_delete( $blog_id . 'short' , 'blog-details' );
wp_cache_delete( md5( $details->domain . $details->path )  , 'blog-lookup' );
wp_cache_delete( 'current_blog_' . $details->domain, 'site-options' );
wp_cache_delete( 'current_blog_' . $details->domain . $details->path, 'site-options' );

5) ...but was missing:

wp_cache_delete( 'get_id_from_blogname_' . $details->blogname, 'blog-details' );

One line patch.

#2 @nacin
12 years ago

Noticing the escaping as well. Are the values escaped on storage, hence the bug report suggesting that all three cache entries are not cleared?

#3 @nacin
12 years ago

  • Owner set to wonderboymusic
  • Status changed from new to assigned

#4 @wonderboymusic
12 years ago

Refreshed

wp_cache_delete( md5( $details->domain . $details->path )  , 'blog-lookup' );

Actually needs to be:

wp_cache_delete( md5( $wpdb->escape( $details->domain ) . $wpdb->escape( $details->path ) )  , 'blog-lookup' );

#5 @wonderboymusic
12 years ago

Only the md5 is not read directly from the object

#6 @ryan
11 years ago

I think get_blog_details( $blog_id, false ) needs to become get_blog_details( $blog_id, true ) to get the blogname.

Last edited 11 years ago by ryan (previous) (diff)

#7 @ryan
11 years ago

get_id_from_blogname( )actually expects a path, but you can't pass get_blog_details()->path directly to it because of trailing slash problems.

@ryan
11 years ago

Rename $name to $slug for some clarity.

#8 @ryan
11 years ago

get_id_from_blogname() actually takes an unslashed path rather than a blogname so I updated the variables and phpdoc to use the term slug instead of name.

  • Rename $name to $slug for some clarity.
  • Strip leading and trailing slashes from the slug in get_id_from_blogname() so get_blog_details()->path could be directly passed
  • Delete the get_id_from_blogname cache in refresh_blog_details()
Last edited 11 years ago by ryan (previous) (diff)

@ryan
11 years ago

Unit tests and blog factory fix

#9 @ryan
11 years ago

Unit tests and a fix to WP_UnitTest_Factory_For_Blog that prepends global $base to the path when creating blogs with generated defaults. get_id_from_blogname() expects "/testpath1/" to be in the DB but the factory was creating "testpath1/".

Last edited 11 years ago by ryan (previous) (diff)

#10 @ryan
11 years ago

Still looking at the blog-lookup cache. In the unit tests sometimes the domain + path passed to get_blog_details() is "example.org" and sometimes "example.org/". I haven't yet looked at what mischief core does during blog creation.

#11 @ryan
11 years ago

The domain and path used for the details blog-lookup key are consistent so far when testing blog create, delete, edit, and load. They're not slashed for the db AFAICT. I haven't tested through signup yet.

#12 @ryan
11 years ago

The current_blog_* cache also appears consistent and not slashed for the DB when testing blog create, delete, edit, and load. This is while testing path based multisite. I haven't tested subdomain yet.

@ryan
11 years ago

More cache tests

#13 @ryan
11 years ago

In [21979]:

  • Invalidate the get_id_from_blogname_* cache in refresh_blog_details()
  • Change $name to $slug in get_id_from_blogname() for some semblance of clarity.
  • Strip leading and trailing slashes from the slug in get_id_from_blogname() so get_blog_details()->path can be passed directly.

Props wonderboymusic
see #20950

#15 @ryan
11 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.