[master] a494a1e27 close a race between VBP_Control() and vbp_thread() wrt VBH index

Nils Goroll nils.goroll at uplex.de
Fri Dec 11 16:43:07 UTC 2020


commit a494a1e2706f5fb642d77352b5285d8e3ea51075
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Fri Dec 11 17:04:59 2020 +0100

    close a race between VBP_Control() and vbp_thread() wrt VBH index
    
    VBP_Control() asserts that, when the vcl is warm, probes are scheduled
    to run (that is, exist on the probe scheduling binheap) and not so
    when the VCL is cold. This is correct because any VCL events are
    guaranteed to be serialized by mgt.
    
    vbp_thread(), however, momentarily left probes removed from the
    binheap while scheduling a probe not holding vbp_mtx, which left a
    window for VBP_Control() to find an active probe not on the binheap.
    
    Both vbp_thread() and vbp_task() re-schedule the probe at hand. The
    former in case scheduling the task fails, and the latter to reschedule
    it at the right interval relative to the probe having finished.
    
    We now re-schedule the probe in vbp_thread() before adding a task to
    run it, which does not change anything about the above, and, in
    particular, keeps vbp_mtx free while adding the probe task.
    
    Yet it closes the race and thus
    
    fixes #3362
    
    survived varnishtest -t 90 -n 10000 -j 200 -i tests/v00003.vtc

diff --git a/bin/varnishd/cache/cache_backend_probe.c b/bin/varnishd/cache/cache_backend_probe.c
index 8d7a9c9b0..5aca2b4bb 100644
--- a/bin/varnishd/cache/cache_backend_probe.c
+++ b/bin/varnishd/cache/cache_backend_probe.c
@@ -487,6 +487,7 @@ vbp_thread(struct worker *wrk, void *priv)
 		} else {
 			VBH_delete(vbp_heap, vt->heap_idx);
 			vt->due = now + vt->interval;
+			VBH_insert(vbp_heap, vt);
 			if (!vt->running) {
 				vt->running = 1;
 				vt->task->func = vbp_task;
@@ -497,7 +498,6 @@ vbp_thread(struct worker *wrk, void *priv)
 				if (r)
 					vt->running = 0;
 			}
-			VBH_insert(vbp_heap, vt);
 		}
 	}
 	NEEDLESS(Lck_Unlock(&vbp_mtx));


More information about the varnish-commit mailing list