Skip to content

Commit

Permalink
Turn enum sess_close into const struct stream_close *
Browse files Browse the repository at this point in the history
More asserts than strictly needed to catch anything I have missed.
  • Loading branch information
bsdphk committed Jan 4, 2022
1 parent 819ed3f commit dbd1028
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 37 deletions.
16 changes: 12 additions & 4 deletions bin/varnishd/cache/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,19 @@ typedef const char *hdr_t;

/*--------------------------------------------------------------------*/

enum sess_close {
SC_NULL = 0,
#define SESS_CLOSE(nm, stat, err, desc) SC_##nm,
#include "tbl/sess_close.h"
struct stream_close {
unsigned magic;
#define STREAM_CLOSE_MAGIC 0xc879c93d
int idx;
unsigned is_err;
const char *name;
const char *desc;
};
extern const struct stream_close SC_NULL[1];
#define SESS_CLOSE(nm, stat, err, desc) \
extern const struct stream_close SC_##nm[1];
#include "tbl/sess_close.h"


/*--------------------------------------------------------------------
* Indices into http->hd[]
Expand Down
18 changes: 14 additions & 4 deletions bin/varnishd/cache/cache_backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ vbe_dir_getfd(VRT_CTX, struct worker *wrk, VCL_BACKEND dir, struct backend *bp,
return (NULL);
}
bo->htc->doclose = SC_NULL;
CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);

FIND_TMO(connect_timeout, tmod, bo, bp);
pfd = VCP_Get(bp->conn_pool, tmod, wrk, force_fresh, &err);
Expand All @@ -176,6 +177,8 @@ vbe_dir_getfd(VRT_CTX, struct worker *wrk, VCL_BACKEND dir, struct backend *bp,
bp->vsc->req++;
Lck_Unlock(bp->director->mtx);

CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);

err = 0;
if (bp->proxy_header != 0)
err += VPX_Send_Proxy(*fdp, bp->proxy_header, bo->sp);
Expand Down Expand Up @@ -229,13 +232,13 @@ vbe_dir_finish(VRT_CTX, VCL_BACKEND d)
CAST_OBJ_NOTNULL(bp, d->priv, BACKEND_MAGIC);

CHECK_OBJ_NOTNULL(bo->htc, HTTP_CONN_MAGIC);
CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);

pfd = bo->htc->priv;
bo->htc->priv = NULL;
if (PFD_State(pfd) != PFD_STATE_USED)
AN(bo->htc->doclose);
if (bo->htc->doclose != SC_NULL || bp->proxy_header != 0) {
VSLb(bo->vsl, SLT_BackendClose, "%d %s close", *PFD_Fd(pfd),
VRT_BACKEND_string(d));
VSLb(bo->vsl, SLT_BackendClose, "%d %s close %s", *PFD_Fd(pfd),
VRT_BACKEND_string(d), bo->htc->doclose->desc);
VCP_Close(&pfd);
AZ(pfd);
Lck_Lock(bp->director->mtx);
Expand Down Expand Up @@ -270,6 +273,8 @@ vbe_dir_gethdrs(VRT_CTX, VCL_BACKEND d)
CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
bo = ctx->bo;
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
if (bo->htc != NULL)
CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);
wrk = ctx->bo->wrk;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CAST_OBJ_NOTNULL(bp, d->priv, BACKEND_MAGIC);
Expand All @@ -283,10 +288,13 @@ vbe_dir_gethdrs(VRT_CTX, VCL_BACKEND d)
http_PrintfHeader(bo->bereq, "Host: %s", bp->hosthdr);

do {
if (bo->htc != NULL)
CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);
pfd = vbe_dir_getfd(ctx, wrk, d, bp, extrachance == 0 ? 1 : 0);
if (pfd == NULL)
return (-1);
AN(bo->htc);
CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);
if (PFD_State(pfd) != PFD_STATE_STOLEN)
extrachance = 0;

Expand All @@ -312,6 +320,7 @@ vbe_dir_gethdrs(VRT_CTX, VCL_BACKEND d)
return (0);
}
}
CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);

/*
* If we recycled a backend connection, there is a finite chance
Expand Down Expand Up @@ -385,6 +394,7 @@ vbe_dir_http1pipe(VRT_CTX, VCL_BACKEND d)
retval = SC_TX_PIPE;
}
V1P_Charge(ctx->req, &v1a, bp->vsc);
CHECK_OBJ_NOTNULL(retval, STREAM_CLOSE_MAGIC);
return (retval);
}

Expand Down
4 changes: 4 additions & 0 deletions bin/varnishd/cache/cache_fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ vbf_stp_retry(struct worker *wrk, struct busyobj *bo)
bo->was_304 = 0;
bo->err_code = 0;
bo->err_reason = NULL;
if (bo->htc != NULL)
bo->htc->doclose = SC_NULL;

// XXX: BereqEnd + BereqAcct ?
VSL_ChgId(bo->vsl, "bereq", "retry", VXID_Get(wrk, VSL_BACKENDMARKER));
Expand Down Expand Up @@ -426,6 +428,8 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)

VSLb_ts_busyobj(bo, "Fetch", W_TIM_real(wrk));
i = VDI_GetHdr(bo);
if (bo->htc != NULL)
CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);

bo->t_resp = now = W_TIM_real(wrk);
VSLb_ts_busyobj(bo, "Beresp", now);
Expand Down
1 change: 1 addition & 0 deletions bin/varnishd/cache/cache_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,7 @@ http_DoConnection(struct http *hp, stream_close_t sc_close)
hp->hdf[v] |= HDF_FILTER;
}
}
CHECK_OBJ_NOTNULL(retval, STREAM_CLOSE_MAGIC);
return (retval);
}

Expand Down
19 changes: 9 additions & 10 deletions bin/varnishd/cache/cache_panic.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,14 @@ boc_state_2str(enum boc_state_e e)

/*--------------------------------------------------------------------*/

const char *
sess_close_2str(stream_close_t sc, int want_desc)
static void
pan_stream_close(struct vsb *vsb, stream_close_t sc)
{
switch (sc) {
case SC_NULL: return (want_desc ? "(null)" : "NULL");
#define SESS_CLOSE(nm, s, err, desc) \
case SC_##nm: return(want_desc ? desc : #nm);
#include "tbl/sess_close.h"

default: return (want_desc ? "(invalid)" : "INVALID");
}
if (sc != NULL && sc->magic == STREAM_CLOSE_MAGIC)
VSB_printf(vsb, "%s(%s)", sc->name, sc->desc);
else
VSB_printf(vsb, "%p", sc);
}

/*--------------------------------------------------------------------*/
Expand Down Expand Up @@ -162,7 +159,9 @@ pan_htc(struct vsb *vsb, const struct http_conn *htc)
return;
if (htc->rfd != NULL)
VSB_printf(vsb, "fd = %d (@%p),\n", *htc->rfd, htc->rfd);
VSB_printf(vsb, "doclose = %s,\n", sess_close_2str(htc->doclose, 0));
VSB_printf(vsb, "doclose = ");
pan_stream_close(vsb, htc->doclose);
VSB_printf(vsb, "\n");
WS_Panic(vsb, htc->ws);
VSB_printf(vsb, "{rxbuf_b, rxbuf_e} = {%p, %p},\n",
htc->rxbuf_b, htc->rxbuf_e);
Expand Down
1 change: 1 addition & 0 deletions bin/varnishd/cache/cache_req_fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1178,6 +1178,7 @@ CNT_Request(struct req *req)
CHECK_OBJ_NOTNULL(wrk->wpriv, WORKER_PRIV_MAGIC);
CHECK_OBJ_ORNULL(wrk->wpriv->nobjhead, OBJHEAD_MAGIC);
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(req->doclose, STREAM_CLOSE_MAGIC);

AN(req->req_step);
AN(req->req_step->name);
Expand Down
58 changes: 46 additions & 12 deletions bin/varnishd/cache/cache_session.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,38 @@ static const struct {
#include "tbl/sess_attr.h"
};

enum sess_close {
SCE_NULL = 0,
#define SESS_CLOSE(nm, stat, err, desc) SCE_##nm,
#include "tbl/sess_close.h"
SCE_MAX,
};

const struct stream_close SC_NULL[1] = {{
.magic = STREAM_CLOSE_MAGIC,
.idx = SCE_NULL,
.is_err = 0,
.name = "null",
.desc = "Not Closing",
}};

#define SESS_CLOSE(nm, stat, err, text) \
const struct stream_close SC_##nm[1] = {{ \
.magic = STREAM_CLOSE_MAGIC, \
.idx = SCE_##nm, \
.is_err = err, \
.name = #nm, \
.desc = text, \
}};
#include "tbl/sess_close.h"

static const stream_close_t sc_lookup[SCE_MAX] = {
[SCE_NULL] = SC_NULL,
#define SESS_CLOSE(nm, stat, err, desc) \
[SCE_##nm] = SC_##nm,
#include "tbl/sess_close.h"
};

/*--------------------------------------------------------------------*/

void
Expand Down Expand Up @@ -512,21 +544,19 @@ SES_Wait(struct sess *sp, const struct transport *xp)
static void
ses_close_acct(stream_close_t reason)
{
int i = 0;

assert(reason != SC_NULL);
switch (reason) {
CHECK_OBJ_NOTNULL(reason, STREAM_CLOSE_MAGIC);
switch (reason->idx) {
#define SESS_CLOSE(reason, stat, err, desc) \
case SC_ ## reason: \
case SCE_ ## reason: \
VSC_C_main->sc_ ## stat++; \
i = err; \
break;
#include "tbl/sess_close.h"

default:
WRONG("Wrong event in ses_close_acct");
}
if (i)
if (reason->is_err)
VSC_C_main->sess_closed_err++;
}

Expand All @@ -541,11 +571,12 @@ SES_Close(struct sess *sp, stream_close_t reason)
{
int i;

assert(reason > 0);
CHECK_OBJ_NOTNULL(reason, STREAM_CLOSE_MAGIC);
assert(reason->idx > 0);
assert(sp->fd > 0);
i = close(sp->fd);
assert(i == 0 || errno != EBADF); /* XXX EINVAL seen */
sp->fd = -(int)reason;
sp->fd = -reason->idx;
ses_close_acct(reason);
}

Expand All @@ -558,6 +589,7 @@ SES_Delete(struct sess *sp, stream_close_t reason, vtim_real now)
{

CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(reason, STREAM_CLOSE_MAGIC);

if (reason != SC_NULL)
SES_Close(sp, reason);
Expand All @@ -575,11 +607,13 @@ SES_Delete(struct sess *sp, stream_close_t reason, vtim_real now)
now = sp->t_open; /* Do not log negatives */
}

if (reason == SC_NULL)
reason = (stream_close_t)-sp->fd;
if (reason == SC_NULL) {
assert(sp->fd < 0 && -sp->fd < SCE_MAX);
reason = sc_lookup[-sp->fd];
}

VSL(SLT_SessClose, sp->vxid, "%s %.3f",
sess_close_2str(reason, 0), now - sp->t_open);
CHECK_OBJ_NOTNULL(reason, STREAM_CLOSE_MAGIC);
VSL(SLT_SessClose, sp->vxid, "%s %.3f", reason->name, now - sp->t_open);
VSL(SLT_End, sp->vxid, "%s", "");
if (WS_Overflowed(sp->ws))
VSC_C_main->ws_session_overflow++;
Expand Down
2 changes: 0 additions & 2 deletions bin/varnishd/cache/cache_varnishd.h
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,6 @@ int PAN__DumpStruct(struct vsb *vsb, int block, int track, const void *ptr,
#define PAN_dump_once_oneline(vsb, ptr, magic, ...) \
PAN__DumpStruct(vsb, 0, 0, ptr, #magic, magic, __VA_ARGS__)

const char *sess_close_2str(stream_close_t sc, int want_desc);

/* cache_pool.c */
void Pool_Init(void);
int Pool_Task(struct pool *pp, struct pool_task *task, enum task_prio prio);
Expand Down
1 change: 1 addition & 0 deletions bin/varnishd/cache/cache_vcl.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ VCL_Req2Ctx(struct vrt_ctx *ctx, struct req *req)

CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(req->doclose, STREAM_CLOSE_MAGIC);

ctx->vcl = req->vcl;
ctx->syntax = ctx->vcl->conf->syntax;
Expand Down
7 changes: 5 additions & 2 deletions bin/varnishd/http1/cache_http1_fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
}

sc = V1L_Close(wrk, &bytes);
CHECK_OBJ_NOTNULL(sc, STREAM_CLOSE_MAGIC);

/* Bytes accounting */
if (bytes < hdrbytes)
Expand All @@ -151,13 +152,15 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
if (sc == SC_NULL && i < 0)
sc = SC_TX_ERROR;

CHECK_OBJ_NOTNULL(sc, STREAM_CLOSE_MAGIC);
if (sc != SC_NULL) {
VSLb(bo->vsl, SLT_FetchError, "backend write error: %d (%s)",
errno, VAS_errtxt(errno));
VSLb(bo->vsl, SLT_FetchError, "backend write error: %d (%s) (%s",
errno, VAS_errtxt(errno), sc->desc);
VSLb_ts_busyobj(bo, "Bereq", W_TIM_real(wrk));
htc->doclose = sc;
return (-1);
}
CHECK_OBJ_NOTNULL(sc, STREAM_CLOSE_MAGIC);
VSLb_ts_busyobj(bo, "Bereq", W_TIM_real(wrk));
return (0);
}
Expand Down
2 changes: 2 additions & 0 deletions bin/varnishd/http1/cache_http1_line.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ V1L_Flush(const struct worker *wrk)
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
v1l = wrk->v1l;
CHECK_OBJ_NOTNULL(v1l, V1L_MAGIC);
CHECK_OBJ_NOTNULL(v1l->werr, STREAM_CLOSE_MAGIC);
AN(v1l->wfd);

assert(v1l->niov <= v1l->siov);
Expand Down Expand Up @@ -252,6 +253,7 @@ V1L_Flush(const struct worker *wrk)
v1l->niov = 0;
if (v1l->ciov < v1l->siov)
v1l->ciov = v1l->niov++;
CHECK_OBJ_NOTNULL(v1l->werr, STREAM_CLOSE_MAGIC);
return (v1l->werr);
}

Expand Down
4 changes: 2 additions & 2 deletions bin/varnishtest/tests/r02219.vtc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ varnish v1 -arg "-p workspace_client=9k" \
} -start

client c1 {
send "PROXY TCP4 127.0.0.1 127.0.0.1 1111 2222\r\nGET /${string,repeat,744,A} HTTP/1.1\r\n\r\n"
send "PROXY TCP4 127.0.0.1 127.0.0.1 1111 2222\r\nGET /${string,repeat,736,A} HTTP/1.1\r\n\r\n"
rxresp
} -run

Expand All @@ -39,6 +39,6 @@ ${string,repeat,732,"42 "}
} -run

client c3 {
send "PROXY TCP4 127.0.0.1 127.0.0.1 1111 2222\r\nGET /${string,repeat,748,C} HTTP/1.1\r\n\r\n"
send "PROXY TCP4 127.0.0.1 127.0.0.1 1111 2222\r\nGET /${string,repeat,740,C} HTTP/1.1\r\n\r\n"
rxresp
} -run
3 changes: 2 additions & 1 deletion include/vrt.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ struct http;
struct lock;
struct req;
struct stevedore;
struct stream_close;
struct suckaddr;
struct vcl;
struct vcldir;
Expand All @@ -268,7 +269,7 @@ struct vsl_log;
struct vsmw_cluster;
struct ws;

typedef enum sess_close stream_close_t;
typedef const struct stream_close *stream_close_t;

/***********************************************************************
* VCL_STRANDS:
Expand Down

0 comments on commit dbd1028

Please sign in to comment.