[master] 81c0e4a Improve client workspace overflow handling.

Lasse Karstensen lkarsten at varnish-software.com
Thu Sep 3 15:42:42 CEST 2015


commit 81c0e4a64e5f5af967324b502661b243ffdbef00
Author: Lasse Karstensen <lkarsten at varnish-software.com>
Date:   Thu Sep 3 15:02:49 2015 +0200

    Improve client workspace overflow handling.
    
    Instead of asserting, return a 500 error to the client if
    running out of client workspace.
    
    This also extends vmod_debug with workspace handling functions, and
    adds two test cases that use these functions.
    
    As discussed at VDD15Q3. Reviewed by Martin.

diff --git a/bin/varnishd/http1/cache_http1_deliver.c b/bin/varnishd/http1/cache_http1_deliver.c
index 324947b..c931971 100644
--- a/bin/varnishd/http1/cache_http1_deliver.c
+++ b/bin/varnishd/http1/cache_http1_deliver.c
@@ -58,6 +58,23 @@ v1d_bytes(struct req *req, enum vdp_action act, void **priv,
 	return (0);
 }
 
+static void
+v1d_error(struct req *req, const char *msg) {
+        static const char r_500[] =
+                "HTTP/1.1 500 Internal Server Error\r\n"
+                "Server: Varnish\r\n"
+                "Connection: close\r\n\r\n";
+
+	VSLb(req->vsl, SLT_Error, "%s", msg);
+	VSLb(req->vsl, SLT_RespProtocol, "HTTP/1.1");
+	VSLb(req->vsl, SLT_RespStatus, "500");
+	VSLb(req->vsl, SLT_RespReason, "Internal Server Error");
+
+	(void)write(req->sp->fd, r_500, sizeof r_500 - 1);
+	req->doclose = SC_TX_EOF;
+	return;
+}
+
 /*--------------------------------------------------------------------
  */
 
@@ -105,6 +122,11 @@ V1D_Deliver(struct req *req, struct busyobj *bo, int wantbody)
 
 	V1L_Reserve(req->wrk, req->ws, &req->sp->fd, req->vsl, req->t_prev);
 
+        if (WS_Overflowed(req->ws)) {
+		v1d_error(req, "workspace_client overflow");
+		return;
+	}
+
 	req->acct.resp_hdrbytes += HTTP1_Write(req->wrk, req->resp, HTTP1_Resp);
 	if (DO_DEBUG(DBG_FLUSH_HEAD))
 		(void)V1L_Flush(req->wrk);
diff --git a/bin/varnishd/http1/cache_http1_line.c b/bin/varnishd/http1/cache_http1_line.c
index 2bcaf6e..04d2533 100644
--- a/bin/varnishd/http1/cache_http1_line.c
+++ b/bin/varnishd/http1/cache_http1_line.c
@@ -80,7 +80,8 @@ V1L_Reserve(struct worker *wrk, struct ws *ws, int *fd, struct vsl_log *vsl,
 
 	res = WS_Snapshot(ws);
 	v1l = WS_Alloc(ws, sizeof *v1l);
-	AN(v1l);
+	if (v1l == NULL)
+		return;
 	INIT_OBJ(v1l, V1L_MAGIC);
 
 	v1l->ws = ws;
@@ -89,9 +90,12 @@ V1L_Reserve(struct worker *wrk, struct ws *ws, int *fd, struct vsl_log *vsl,
 	u = WS_Reserve(ws, 0);
 	u = PRNDDN(u);
 	u /= sizeof(struct iovec);
-	if (u > IOV_MAX)
+	if (u == 0) {
+		WS_Release(ws, 0);
+		WS_MarkOverflow(ws);
+		return;
+	} else if (u > IOV_MAX)
 		u = IOV_MAX;
-	AN(u);
 	v1l->iov = (void*)PRNDUP(ws->f);
 	v1l->siov = u;
 	v1l->ciov = u;
@@ -102,6 +106,7 @@ V1L_Reserve(struct worker *wrk, struct ws *ws, int *fd, struct vsl_log *vsl,
 	v1l->t0 = t0;
 	v1l->vsl = vsl;
 	wrk->v1l = v1l;
+	return;
 }
 
 unsigned
diff --git a/bin/varnishtest/tests/c00070.vtc b/bin/varnishtest/tests/c00070.vtc
new file mode 100644
index 0000000..27f339f
--- /dev/null
+++ b/bin/varnishtest/tests/c00070.vtc
@@ -0,0 +1,55 @@
+varnishtest "Test workspace functions in vmod_debug"
+
+server s1 {
+	rxreq
+	txresp
+
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl+backend {
+	import ${vmod_debug};
+	sub vcl_backend_response {
+		set beresp.http.free_backend = debug.workspace_free(backend);
+	}
+
+	sub vcl_deliver {
+		set resp.http.free_client = debug.workspace_free(client);
+		set resp.http.free_session = debug.workspace_free(session);
+		set resp.http.free_thread = debug.workspace_free(thread);
+
+		set resp.http.overflowed = debug.workspace_overflowed(client);
+		debug.workspace_allocate(client, 2048);
+
+		if (req.url == "/bar") {
+			debug.workspace_overflow(client);
+		}
+	}
+} -start
+
+logexpect l1 -v v1 -d 1 -g vxid -q "Error ~ 'overflow'" {
+        expect 0 *      Begin
+        expect * =      Error	"workspace_client overflow"
+        expect * =      End
+} -start
+
+client c1 {
+	txreq -url /foo
+	rxresp
+	expect resp.http.overflowed == "false"
+
+	expect resp.http.free_client > 57000
+	expect resp.http.free_backend > 57000
+	expect resp.http.free_session > 250
+	expect resp.http.free_thread > 2000
+} -run
+
+client c2 {
+	txreq -url /bar
+	rxresp
+	expect resp.status == 500
+} -run
+
+logexpect l1 -wait
+
diff --git a/bin/varnishtest/tests/c00071.vtc b/bin/varnishtest/tests/c00071.vtc
new file mode 100644
index 0000000..59e3281
--- /dev/null
+++ b/bin/varnishtest/tests/c00071.vtc
@@ -0,0 +1,47 @@
+varnishtest "Test actual client workspace overflow"
+
+server s1 {
+	rxreq
+	txresp
+
+	rxreq
+	txresp
+
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl+backend {
+	import ${vmod_debug};
+	sub vcl_deliver {
+		debug.workspace_allocate(client, debug.workspace_free(client) - 200);
+
+		if (req.url ~ "/bar") {
+			set resp.http.x-foo = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
+		}
+		else if (req.url ~ "/baz") {
+			set resp.http.x-foo = regsub(req.url, "baz", "baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaz");
+		}
+		set resp.http.x-of = debug.workspace_overflowed(client);
+	}
+} -start
+
+client c1 {
+	txreq -url /foo
+	rxresp
+	expect resp.status == 200
+	expect resp.http.x-of == "false"
+
+	txreq -url /bar
+	rxresp
+	expect resp.status == 500
+	expect resp.http.x-of == <undef>
+} -run
+
+client c2 {
+	txreq -url /baz
+	rxresp
+	expect resp.status == 500
+	expect resp.http.x-of == <undef>
+} -run
+
diff --git a/lib/libvmod_debug/vmod.vcc b/lib/libvmod_debug/vmod.vcc
index 8b32e92..dad36ec 100644
--- a/lib/libvmod_debug/vmod.vcc
+++ b/lib/libvmod_debug/vmod.vcc
@@ -122,3 +122,19 @@ Return the dynamic backend.
 $Method VOID .refresh(STRING addr, STRING port)
 
 Dynamically refresh & (always!) replace the backend by a new one.
+
+$Function VOID workspace_allocate(ENUM { client, backend, session, thread }, INT SIZE)
+
+Allocate and zero out SIZE bytes from a workspace.
+
+$Function BOOL workspace_overflowed(ENUM { client, backend, session, thread })
+
+Return if the workspace overflow mark is set or not.
+
+$Function VOID workspace_overflow(ENUM { client, backend, session, thread })
+
+Mark a workspace as overflowed.
+
+$Function INT workspace_free(ENUM { client, backend, session, thread })
+
+Find how much unallocated space there is left in a workspace.
diff --git a/lib/libvmod_debug/vmod_debug.c b/lib/libvmod_debug/vmod_debug.c
index c9b1028..732d89a 100644
--- a/lib/libvmod_debug/vmod_debug.c
+++ b/lib/libvmod_debug/vmod_debug.c
@@ -290,3 +290,72 @@ vmod_sleep(VRT_CTX, VCL_DURATION t)
 	CHECK_OBJ_ORNULL(ctx, VRT_CTX_MAGIC);
 	VTIM_sleep(t);
 }
+
+static struct ws *wsfind(VRT_CTX, VCL_ENUM which) {
+	if (!strcmp(which, "client")) {
+		return ctx->ws;
+	} else if (!strcmp(which, "backend")) {
+		return ctx->bo->ws;
+	} else if (!strcmp(which, "session")) {
+		return ctx->req->sp->ws;
+	} else if (!strcmp(which, "thread")) {
+		return ctx->req->wrk->aws;
+	} else
+		WRONG("No such workspace.");
+}
+
+void
+vmod_workspace_allocate(VRT_CTX, VCL_ENUM which, VCL_INT size)
+{
+	struct ws *ws;
+	char *s;
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+	ws = wsfind(ctx, which);
+
+	WS_Assert(ws);
+	AZ(ws->r);
+
+	s = WS_Alloc(ws, size);
+	if (!s)
+		return;
+	memset(s, '\0', size);
+}
+
+VCL_INT
+vmod_workspace_free(VRT_CTX, VCL_ENUM which)
+{
+	struct ws *ws;
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+	ws = wsfind(ctx, which);
+
+	WS_Assert(ws);
+	AZ(ws->r);
+
+	return pdiff(ws->f, ws->e);
+}
+
+VCL_BOOL
+vmod_workspace_overflowed(VRT_CTX, VCL_ENUM which)
+{
+	struct ws *ws;
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+	ws = wsfind(ctx, which);
+	WS_Assert(ws);
+
+	return (WS_Overflowed(ws));
+}
+
+void
+vmod_workspace_overflow(VRT_CTX, VCL_ENUM which)
+{
+	struct ws *ws;
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+	ws = wsfind(ctx, which);
+	WS_Assert(ws);
+
+	WS_MarkOverflow(ws);
+}



More information about the varnish-commit mailing list