[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