WordPress.org

Make WordPress Core

Opened 4 weeks ago

Last modified 4 weeks ago

#44445 new defect (bug)

wp_cache_init() and WP_Object_Cache constructor has a memory leak

Reported by: MikeSchinkel Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.9.6
Component: Cache API Keywords: dev-feedback
Focuses: performance Cc:

Description

When calling wp_cache_init() repeated in unit testing the WP_Object_Cache::contruct() repeatedly registers 'destruct' as a shutdown function, and each time it does it leaks memory.

There is a @todo comment above the register_shutdown_hook() that says the following so I would assume that this is no longer needed and we could just delete the line with the register_shutdown_hook()?

This should be moved to the PHP4 style constructor, PHP5 already calls destruct()

I will upload a patch to delete the list, and a different patch to only call register_shutdown_hook() once, depending on what is appropriate.

Attachments (3)

44445-remove-call.patch (676 bytes) - added by MikeSchinkel 4 weeks ago.
This patch removes the call to register_shutdown_function().
44445-restrict-call.patch (923 bytes) - added by MikeSchinkel 4 weeks ago.
This patch restricts the call to register_shutdown_function() to be called only once.
44445-remove-all.patch (846 bytes) - added by ayeshrajans 4 weeks ago.
Completely removes the dead __destruct and register_shutdown_function call.

Download all attachments as: .zip

Change History (4)

@MikeSchinkel
4 weeks ago

This patch removes the call to register_shutdown_function().

@MikeSchinkel
4 weeks ago

This patch restricts the call to register_shutdown_function() to be called only once.

#1 @ayeshrajans
4 weeks ago

  • Focuses performance added
  • Keywords dev-feedback added

The destructor simply returns true and doesn't clean up anything that could've prevented the said memory leak. Even the oldest state of cache.php file I could find (https://github.com/WordPress/wordpress-develop/blob/b43712e0f79a9f5bea52217e06155e2f471c490c/src/wp-includes/cache.php, oldest checkout available 5 years ago) has the exact same code. I'm pretty sure this is dead code, which we should probably remove from the root.

With the __destruct and shut down function registration removed, the test suite still passes: https://travis-ci.org/Ayesh/wordpress-develop/builds/396071330

I will attach the patch for reference.

With the patch applied, PHP 7.1 test suite completed the execution and memory usage statistics from PHPUnit are identical when compared to a Travis results before the patch (https://travis-ci.org/Ayesh/wordpress-develop/jobs/396068504)

@ayeshrajans
4 weeks ago

Completely removes the dead __destruct and register_shutdown_function call.

Note: See TracTickets for help on using tickets.