Make sense from my point of view!

this seems reasonable to me. the other kernel events are also not triggered on a cache hit (as the cache kernel does not forward the request to the wrapped kernel). having only the terminate event seems odd. and indeed, when a separate caching proxy is used, no terminate is triggered.

the code was added over a decade ago:7efe4bc

i can see the fear of breaking some existing systems. i think most delayed tasks should not be needed after a cache hit, but if some sort of reporting is done in the framework it might care about cache hits too.

another option could be to use a flag whether to forward the termination event for cache hits that defaults to true for now, and maybe do a deprecation warning if it is not set to false, to make people aware that the behaviour will change.

without a flag, the behaviour can be changed by extending the HttpCache class and overwriting the terminate method to unconditionally forward the event.

regarding bug vs change, i think we are here in really a gray area.

there is nothing technically wrong with calling terminate. no security risk, no exceptions happening. you can't be sure that nobody would want the event to happen, so i don't think its correct to consider it a bug.

it is a significant waste of server resources even if it should not affect response times directly. the option seems justified to me, and given how unlikely it is to need the event, i think it makes sense to change the default in the next major version. (e.g. logging of requests should happen at webserver level, not in PHP)

given how long it has been like this, i would vote for stability in this case and only change the default in the next major.

As explained in the comments, this must be done in 6.2.
In addition, performance optimizations are only for 6.2 as well.
Also, as it might break BC anyway, it cannot be part of 4.4 where stability is paramount.

With the option there is no BC break. It just offers the users the ability to adjust their setups in 4.4 already. I'm unsure what I need to do now :) If you want this in 6.2 only, would you want me to still keep the option when I rebase or not? :)

We really appreciate the investigations. The proposed solution is good for the next minor version (6.2).@fabpot listed the reasons it cannot be merged in an old branch:

• it is not a security issue, not a functional bug
• it is a new feature (performance optimization are feature)
• it adds a new option
• it adds a depreciation message (which is good for changing the default behavior in 7.0)