[wp-trac] [WordPress Trac] #20635: _pad_term_count get's stuck if there is a loop in the hierarchy

[WordPress Trac]

#20635: _pad_term_count get's stuck if there is a loop in the hierarchy
-----------------------------------------------+-----------------------
 Reporter:  westi                              |       Owner:  westi
     Type:  defect (bug)                       |      Status:  accepted
 Priority:  normal                             |   Milestone:  4.2
Component:  Taxonomy                           |     Version:
 Severity:  normal                             |  Resolution:
 Keywords:  westi-likes has-patch 2nd-opinion  |     Focuses:
-----------------------------------------------+-----------------------
Changes (by boonebgorges):

 * keywords:  westi-likes has-patch 3.6-early => westi-likes has-patch 2nd-
     opinion
 * milestone:  Future Release => 4.2


Comment:

 Replying to [comment:9 nacin]:
 > How expensive is this? If done on the front page of a site when
 something is otherwise a read operation, are we looking at a race to run
 the wp_update_term() to fix the loop in the hierarchy?
 >
 > westi, what did you mean by "stuck"? Infinite loop? Or something less
 bad?

 Appears to be an infinite `while` loop.

 `wp_check_term_hierarchy_for_loops()` is more than we need in this case.
 (a) It's probably overly expensive to break the hierarchy on a read
 operation like `get_terms()`, when all we really need here is to ensure
 that we don't loop infinitely; and (b) there's no need to recurse the
 hierarchy using `get_term()` (`wp_get_term_taxonomy_parent_id()`) because
 `_pad_term_counts()` does a single query to grab the whole hierarchy -
 that is, we already have all the data we need to detect a loop.

 I propose that we keep track of already-identified `$ancestors` during the
 `while` loop, and just break out of it if we detect a loop. Worst case
 scenario with this fix is that, if the loop comprises only a subset of the
 terms identified in the `get_terms()` query, it's possible that some terms
 won't have their counts padded. See [attachment:20635.3.diff] (note that
 the unit test won't outright fail without the fix, but you'll probably get
 a maximum execution time fatal error).

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