[wp-trac] [WordPress Trac] #17817: do_action/apply_filters/etc. recursion on same filter kills underlying call

WordPress Trac noreply at wordpress.org
Thu Sep 8 13:00:30 UTC 2016


#17817: do_action/apply_filters/etc. recursion on same filter kills underlying call
----------------------------------------------------+---------------------
 Reporter:  kernfel                                 |       Owner:  pento
     Type:  defect (bug)                            |      Status:  closed
 Priority:  normal                                  |   Milestone:  4.7
Component:  Plugins                                 |     Version:  2.2
 Severity:  normal                                  |  Resolution:  fixed
 Keywords:  has-patch needs-testing has-unit-tests  |     Focuses:
----------------------------------------------------+---------------------

Comment (by jacobsantos):

 Replying to [comment:220 noplanman]:
 > Ok, I see what you mean.
 >
 > I'm just curious to know if recursive adding/removing of hooks within a
 hook would work properly then, as the array would be changing during the
 loop.
 >
 > At the moment I'm just happy that @pento has put in the effort to get
 this fixed.

 Yeah, that was it exactly. The problem is what do you want to happen? If
 you add a callback to a hook that is already running, then what do you
 want to happen?

  1. The callback is called at the next time the hook is called.
  1. The callback is called later within the same hook execution. So if you
 add a callback to hook 'something' running with priority 10, while running
 at priority 10, that same callback is called at some point soon after.

 The solution can't have both. Or realistically, it could have both, but
 solutions for both are different and counter each other. I am sure this
 was mentioned at some point above. With the patch, great, I am guessing
 the patch is the option 2.

 The problem is what do people mean when they add to the same hook they are
 running on? So if the programmer starts adding callbacks to the 'wp_init'
 hook, do they mean they want it to run soon after? I understand there is a
 chicken and the egg problem. That is why there is an early process startup
 execution and late process startup execution hook. So you can manage what
 goes into the latter in the former.

 The point is more so that if you want to remove a callback from a hook,
 you may do so. What logical reason does a person have for adding more
 hooks to 'wp_init'? Do you mean you want to execute the callbacks? Why not
 just execute the callbacks?

 ------------

 For me, the problem is not that the problem should or should not have been
 fixed. Incorrect behavior is incorrect behavior and should be fixed. The
 problem is that the solution required is hideous. Not the code provided
 and the code required to accurately and completely fix the problem. The
 problem is fundamental with how PHP handles arrays and references and
 pointers. Requiring code that breaks this behavior in some way to provide
 consistency.

 This is code that when it breaks, it will be damn near impossible to fix.
 It will be damn near impossible to optimize. The original patch like the
 one committed was beautiful in that only one person knew how it worked.
 Only one or a few people would ever know how it worked. You do not want
 code like that in any project. You may think you understand how this code
 works, but you don't. Or at least, if it was like the original patch. The
 code that was committed appears to be a bit more straightforward, so I am
 not sure if that just means I know more or that the author is a better
 code artist.

 I only got to the point where what appears to be an incredibly expensive
 method is called, each time a callback is added to a hook. Sure,
 optimization is sometimes counter-intuitive, but when has sorting ever
 been a fast process? For each addition? Insanity. I would buy a hat and
 eat it, if sorting each time didn't have a huge performance hit.

 It is said that copying the array would have performance issues. How does
 what was committed not have huge performance issues?

 The effort is awesome, sure, but you just used C4 to clean a window. I
 guess technically, the window is clean, albeit scattered to the wind.

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


More information about the wp-trac mailing list