• Nils Goroll's avatar
    Add missing bits for full streaming support · 97578b06
    Nils Goroll authored
    Previous code would only deliver fully received segments even if a
    busy object was being written to by the backend side (streaming).
    
    I guess at some point before the public release I must have thought
    about this and decided that streaming only completed segments should
    be acceptable at least to begin with, but I overlooked the fact that
    the previous implementation could lead to short body writes due to a
    lack of coordination between the writing and reading side.
    
    This commit introduces proper streaming support, also within busy
    segments.
    
    In particular, this should also solve an issue reported by @tomazz75
    on gitlab where no body was sent at all. This problem could lead to
    the client stalling on a wait for body data which never came.
    
    The nature of this problem was a race condition, which I was only able
    to reproduce on my system with the following patch to favor the race:
    
    diff --git a/src/fellow_cache.c b/src/fellow_cache.c
    index 3073b35..c9d9a4e 100644
    --- a/src/fellow_cache.c
    +++ b/src/fellow_cache.c
    @@ -3338,6 +3338,9 @@ fellow_busy_obj_getspace(struct fellow_busy *fbo, size_t *sz, uint8_t **ptr)
            assert(*sz > 0);
            AN(ptr);
    
    +       // XXX DEBUG
    +       usleep(10000);
    +
            CHECK_OBJ_NOTNULL(fbo->fc, FELLOW_CACHE_MAGIC);
            CHECK_OBJ_NOTNULL(fbo->fc->tune, STVFE_TUNE_MAGIC);
            max = (size_t)1 << fbo->fc->tune->chunk_exponent;
    
    This fix survived 1000 calls to the respective test case:
    
    	/tmp/bin/varnishtest \
    		-Dlibvmod_slash=/tmp/lib/varnish/vmods/libvmod_slash.so \
    		src/vtc/fellow_c00093.vtc \
    		-n 1000 -j 20
    
    Thank you to @tomazz75 for reporting the bug and help with additional
    information to track it down.
    
    Fixes #2
    97578b06
fellow_c00093.vtc 1.18 KB