I'm a new developer/user for Varnish, and was looking to extend a module
with some regex support (vmod_cookie if you must know). I don't know
anything about vmod development or Varnish threading, so to get an idea of
how to do this I looked at the vmod_header module.
But I'm confused about why they thought they needed a mutex to protected a
call to VRT_re_init. Is this just a useless, historic thing that got left
in for some reason?
https://github.com/varnish/varnish-modules/blob/master/src/vmod_header.c
If you look at vmod_remove it has a 2nd parameter that's in the vcc as
PRIV_CALL. This parameter is initialized with the string value from the
last parameter (a regex pattern). It does this by calling header_init_re().
And if you look at that function you see:
/*
* Initialize the regex *s on priv, if it hasn't already been done.
* XXX: We have to recheck the condition after grabbing the lock to avoid a
* XXX: race condition.
*/
static void
header_init_re(struct vmod_priv *priv, const char *s)
{
if (priv->priv == NULL) {
assert(pthread_mutex_lock(&header_mutex) == 0);
if (priv->priv == NULL) {
VRT_re_init(&priv->priv, s);
priv->free = VRT_re_fini;
}
pthread_mutex_unlock(&header_mutex);
}
}
My reading of the docs makes me think that this "priv" only lives for each
single call of the function. There should be no reason to protect it with a
global mutex. Do the internals of VRT_re_init() require this? I can't
imagine how, because if it did then this method of protecting it is broken
anyway.
I suspect this is leftover from some rewrite or updating of the module but
wanted to check.
with some regex support (vmod_cookie if you must know). I don't know
anything about vmod development or Varnish threading, so to get an idea of
how to do this I looked at the vmod_header module.
But I'm confused about why they thought they needed a mutex to protected a
call to VRT_re_init. Is this just a useless, historic thing that got left
in for some reason?
https://github.com/varnish/varnish-modules/blob/master/src/vmod_header.c
If you look at vmod_remove it has a 2nd parameter that's in the vcc as
PRIV_CALL. This parameter is initialized with the string value from the
last parameter (a regex pattern). It does this by calling header_init_re().
And if you look at that function you see:
/*
* Initialize the regex *s on priv, if it hasn't already been done.
* XXX: We have to recheck the condition after grabbing the lock to avoid a
* XXX: race condition.
*/
static void
header_init_re(struct vmod_priv *priv, const char *s)
{
if (priv->priv == NULL) {
assert(pthread_mutex_lock(&header_mutex) == 0);
if (priv->priv == NULL) {
VRT_re_init(&priv->priv, s);
priv->free = VRT_re_fini;
}
pthread_mutex_unlock(&header_mutex);
}
}
My reading of the docs makes me think that this "priv" only lives for each
single call of the function. There should be no reason to protect it with a
global mutex. Do the internals of VRT_re_init() require this? I can't
imagine how, because if it did then this method of protecting it is broken
anyway.
I suspect this is leftover from some rewrite or updating of the module but
wanted to check.