[wp-trac] [WordPress Trac] #38285: Object lookups by ID (or primary key) are not managed properly, lots of memory leaks

[WordPress Trac]

#38285: Object lookups by ID (or primary key) are not managed properly, lots of
memory leaks
----------------------------+-----------------------------
 Reporter:  wonderboymusic  |      Owner:
     Type:  defect (bug)    |     Status:  new
 Priority:  normal          |  Milestone:  Awaiting Review
Component:  Cache API       |    Version:
 Severity:  normal          |   Keywords:
  Focuses:                  |
----------------------------+-----------------------------
 tl;dr this affects every object that uses the `WP_Post::get_instance()`
 paradigm of "managing instances" (spoiler: they are not)

 `WP_Post` and friends are supposed to add more sanity to caching and
 sanitization. They are not models, they are read-only data objects that
 strongly-type database rows. As such, it should be easy to maintain only
 one reference to any given row = one object per row, regardless of method
 of query, you should always get the exact same object. This tends to work
 if you pass around objects, but not when passing around IDs.

 Passing around IDs is a common use-case:

 {{{
 $ids = get_favorite_posts();

 foreach ( $ids as $id ) {
     $post = get_post( $id );

     // wreak havoc
 }
 }}}

 In a theme:

 {{{
 get_the_title( get_the_ID() );
 }}}

 In Doctrine (or Hibernate, or any other ORM), each type is an Entity that
 is managed by an Entity Manager. There is never more than one instance of
 a single item. In WordPress, we get `get_post()` and a built-in non-
 persistent Array Cache. We also get `WP_Post::get_instance( $id )`, which
 makes us believe that `WP_Post` is managing instances. It is not. The
 Array Cache is maintaining all of these references.

 A few problems:
 * calls to `::get_instance()` will always touch the cache
 * The Array Cache clones objects before storing them, and clones objects
 before returning them
 * The Array Cache is storing `stdClass` instances, not `WP_Post` instance,
 so every item out of the cache also has to become a brand new instance of
 `WP_Post`.

 Storing the items as `WP_Post` objects would seemingly remove the need to
 return new instances, but they would be new instances nonetheless, since
 the cache clones every object before saving / returning.

 What needs to be done:
 * Object rows are stored strongly-typed
 * Cache does not clone objects

 How to test that this is happening (using WP-CLI):

 {{{
 <?php
 class Prof {
         public function objects() {
                 $store = new \SplObjectStorage();

                 $mem1 = memory_get_usage();

                 $p = get_post( 27 );
                 foreach ( range( 1, 20 ) as $i ) {
                         $p = get_post( $p );
                         if ( ! $store->contains( $p ) ) {
                                 $store->attach( $p );
                         }
                 }

                 $mem2 = memory_get_usage();

                 \WP_CLI::line( round( ( $mem2 - $mem1 ) / 1024, 2 ) );
                 \WP_CLI::line( $store->count() );
         }

         public function ids() {
                 $store = new \SplObjectStorage();

                 $mem1 = memory_get_usage();

                 foreach ( range( 1, 20 ) as $i ) {
                         $p = get_post( 27 );
                         if ( ! $store->contains( $p ) ) {
                                 $store->attach( $p );
                         }
                 }

                 $mem2 = memory_get_usage();

                 \WP_CLI::line( round( ( $mem2 - $mem1 ) / 1024, 2 ) );
                 \WP_CLI::line( $store->count() );
         }
 }

 if ( class_exists( '\WP_CLI' ) ) {
         \WP_CLI::add_command( 'prof', Prof::class );
 }
 }}}

 Both should print out 1. Without the patch, `ids` will print out 20 + a
 bunch of leaked memory.

 Patch for `WP_Post` + `WP_Object_Cache` attached

--
Ticket URL: <https://core.trac.wordpress.org/ticket/38285>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform