varnishcache / varnish-cache

Varnish Cache source code repository
https://www.varnish-cache.org
Other
3.56k stars 366 forks source link

Avoid stale PRIV_{TASK,TOP} pointers by calling VRT_priv_{task,top} for each such argument #4066

Closed nigoroll closed 4 months ago

nigoroll commented 4 months ago

Description mostly identical to that of the second commit:

Before this change, we would call VRT_priv_* only once per subroutine, which can be *) a nice performance optimization, but leaves us with stale pointers after a rollback.

Rather than adding complications for the rollback case just to keep the option of the "per subroutine pointer cache", just retrieve a fresh priv pointer every time.

The other use of the per subroutine initialization was error handling, which needs additional code outside the function arguments, simply because a return statement is not possible within function arguments. We remove the requirement for error handling by making sure that VRT_priv_{task,top} always return a valid pointer: They now fall back to a heap allocation if the workspace is full, which is unlikely and should not impact the happy path. In other words, the fallback only exists to avoid the need for error handling.

Fixes https://github.com/varnish/varnish-modules/issues/222

Alternative implementation to #4060


*) It is not an optimization in all cases, for example the priv pointers were intialized unconditionally, even if code using them was not reached - but then again, this is something C compilers might optimize...