When we introduced checking ESI includes for status 200 or 204, we
would only (potentially, depending on the onerror attribute and
esi_onerror_continue parameter) abort ESI processing for includes
with a non-zero length.

This patch makes behavior consistent to not depend on whether or not an
include is empty.

Fixes #4070

Ref 2d494dc7
    bb3d027b

We no longer need to mark used vmods in subroutines (see previous
commit)

to avoid using stale pointers after a rollback.

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
removed the requirement for error handling in the previous commit by
making sure that VRT_priv_{task,top} always return a valid pointer.

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...

this is in preparation of a follow up commit

This reverts the code change from commit 6d423aa5,
keeps the test case and adds a VDP_Close() for the case that the
transport deliver function is not called.

Part 2 of the fix for #4067

Conflicts:
	bin/varnishd/cache/cache_req_fsm.c

Part 1 of the fix for #4067

this is for consistency, to simplify parsing and to reduce the amount of
data logged with VSL.

Motivated by #4042

Signed-off-by: Asad Sajjad Ahmed <asadsa@varnish-software.com>

Refs #4063

Very much looks like copy-pasta in ede8c3db

JAIL_FIXFD_VSMMGT and JAIL_FIXFD_VSMWRK work on directories but,
as the name implies, JAIL_FIXFD_FILE not.

This adds coverage for a non-delivery transition from vcl_hit, where the
busy objcore would drop its sole reference in the event of a grace hit.
The lack of coverage was visible in the gcov dashboard:

    669   580   if (busy != NULL) {
    670   0     	(void)HSH_DerefObjCore(wrk, &busy, 0);
    671   0     	VRY_Clear(req);
    672   0     }

There should now be at least one pass inside this block.

Refs #4032

Refs #4055

When a VMOD adds a CFLAG with `libvmod_$1_la_CFLAGS` the object name is no
longer `vmod_$name.lo` but `$library-vmod_name.lo` this changes the
build order such that the VCC autogenerated files would not be
guaranteed to compile first causing compilation issues.

For chunked encoding, we do not know how big the object is ultimately
going to be, so VFP_GetStorage() called ObjGetSpace() with the
fetch_chunksize parameter in this case.

Yet which size is best might differ for different storage engines, and
having the information that the caller does not know the final size
might be relevant. Storage engines could guess that if a request came
in for fetch_chunksize that this _might_ be the "chunked" case, but
that heuristic would be wrong for Objects of just that size advertised
via Content-Length.

So this patch takes the guesswork out of the game by just passing the
magic 0 value down to the storage engine to mean "give me some good
chunk of bytes, I do not know how much I am going to need".

Since, we seems to not use it at all.
Signed-off-by: Asad Sajjad Ahmed <asadsa@varnish-software.com>

When `vcl_deliver` does not return `deliver` the timestamp ends
up being a duplicate of the `synth` equivalent or the `restart`
timestamp.

Instead of adding hoops to the documentation, in particular to keep it
in sync, improve the only location where we emit HTC status logs.

We could also consider replacing the HTC enum with a struct, similar to
what we did in other places. The struct symbols would be named after the
UPPER name from the table, have a name and description fields, possibly
replace the error number with a simple is_err field.

Capturing the long description like this is less intrusive.

Refs #4042

Spotted by Darryl Rodden.

Submitted by: @AlveElde in Vtest repo on github

This is an extended variable that has the same format as the default one
used when no format is specified.  The variable is useful for appending
or prepending other formatters to the default format.

If VRT_AddDirector() was called from handling a VCL_COLD event or,
indirectly, from another thread which the VCL_COLD event handler was
waiting for, varnishd would deadlock and prevent any CLI or director
changes, because VRT_AddDirector() requires the vcl_mtx, which is held
during vcl_BackendEvent() to ensure a consistent view of the director
list. Because of the early return from VRT_AddDirector() this likely
only happened in VTC mode, but the underlying race existed
nevertheless.

This patch _almost_ fixes the issue with the intend of making it
highly unlikely to occur without getting too involved with the vcl
temperature controls: We now check the same conditions under which
vcl_set_state() would transition the temperature to COOLING and, if
they apply, use Lck_Trylock() in a try/wait loop instead of
Lck_Lock(), avoiding the deadlock.

The patch presumably still does not fix the problem entirely, because
the reads of vcl->busy and vcl->temp before the Lck_Trylock() could
still be outdated. With the temperature controls otherwise unchanged,
the only alternative idea I could come up with was to always use a
try/wait loop, which I dismissed due to the performance impact
(overhead and added latency).

Ref https://github.com/nigoroll/libvmod-dynamic/issues/110

Otherwise poll(2) returns EINVAL on FreeBSD when both pipe_timeout and
pipe_task_deadline are disabled.

Fixes #4043

It takes a vtim_dur and returns a number of milliseconds. NAN means no
timeout and negative values are considered expired deadlines and get a
chance to succeed a non-blocking poll.

This test case has been failing for a while in continuous integration
and trying to bisect the change that triggered a stack overflow for our
Ubuntu 18.04 job I wasn't able to make it pass, even for the commit
where it first landed.

Something must have happened outside of Varnish to consume more stack.

The other timeouts should eventually become unset-able as well, where
an unset variable falls back to a parameter and zero means no timeout,
overriding the fallback parameter.

Refs #4043

... and minor touch up of the file.

No, this does not imply me checking all our RST files for adherence with
this standard.

Ref 0d663a5e

The [be]req.task_deadline should eventually be generalized to all
subroutines with read-write access to [be]req. When that happens,
bereq.task_deadline will inherit req.task_deadline during the pipe
transition.

Related tasks should otherwise have independent deadlines, except
sub-request tasks like ESI where there is a hierarchical dependency.

This takes care of documenting the meaning of a zero timeout value in
a central location, since none of the timeout parameters have a mention
for the effect it produces.

This is written as if there was a separate varnish-param(7) manual,
which should eventually be enacted. This manual extraction was initially
implemented in the adjacent #3817 timeout nomenclature draft.