Hi!
I've attached a new patch implementing fileread with the required
corrections (std-vmod-fileread.diff) and a patch implementing a basic
test-case (fileread-test-case.diff) - I am still figuring out how to
properly write a real multi-threaded stress-test.
There are a few issues, however. Firstly, since the file is being
mmaped, files whose sizes are exact multiples of the system page size
will cause a segmentation fault. This is a direct consequence of the
fact that VCL strings are null terminated - unless there is some
(zeroed, by POSIX) space _left over_ due to a non-aligned file-size,
code that tries to read the string will end up trying to access a
non-mapped memory location.
As I see it (from within the confines of my limited experience :) ),
there are three ways by which this can be corrected:
i. Don't process files with sizes which are evenly divided by the
system page size. This is what is done now. This really does not solve
anything.
ii. Open file, copy to memory, close file. This may be too
inefficient, but this solves the second problem as well (mentioned
below).
iii. Instead of representing VCL strings as regular null-terminated C
strings, store the length somewhere; perhaps by representing them as a
struct containing a pointer to a character array and a length. When
required (for passing them to standard library functions, for
instance), they can be trivially converted to a regular C string, by
copying.
The second issue is that of the file changing after it has been
mmapped. While on Linux it seems that the changes are reflected back
to mmaped memory block, this is not a scenario well defined by POSIX.
Even in Linux, we're in trouble if a file which was not originally of
a length evenly divisible by the page size becomes so due to the
addition of text. One solution, as I see it, would be to add another
function, calling it filerefresh, which remaps (with the new length)
the file whose file-name is passed, possibly after doing a timestamp
check. This effectively leaves it to the VCL writer to decide which
files may change, and which may not.
--
Regards,
Sanjoy Das.
http://playingwithpointers.com
Public Key at http://playingwithpointers.com/custom/public_key.txt
Index: varnish/varnish-cache/bin/varnishtest/tests/m00003.vtc
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ varnish/varnish-cache/bin/varnishtest/tests/m00003.vtc 2010-11-11 17:52:53.000000000 +0530
@@ -0,0 +1,25 @@
+test "Test std.fileread"
+
+shell "echo 'Some Random Text' > ${tmpdir}/fileread.test"
+
+server s1 {
+ rxreq
+ txresp -proto HTTP/1.0 -nolen -bodylen 9
+} -start
+
+varnish v1 -vcl+backend {
+ import std from "${topbuild}/lib/libvmod_std/.libs/libvmod_std.so.1" ;
+
+ sub vcl_deliver {
+ set resp.http.data = std.fileread("${tmpdir}/fileread.test");
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.http.data == "Some Random Text"
+ expect resp.bodylen == 9
+}
+
+client c1 -start
Index: varnish/varnish-cache/lib/libvmod_std/vmod.vcc
===================================================================
--- varnish.orig/varnish-cache/lib/libvmod_std/vmod.vcc 2010-11-11 16:42:11.000000000 +0530
+++ varnish/varnish-cache/lib/libvmod_std/vmod.vcc 2010-11-11 16:42:25.000000000 +0530
@@ -33,3 +33,4 @@
Function REAL random(REAL, REAL)
Function VOID log(STRING_LIST)
Function VOID syslog(INT, STRING_LIST)
+Function STRING fileread(PRIV_CALL, STRING)
Index: varnish/varnish-cache/lib/libvmod_std/vmod_std.c
===================================================================
--- varnish.orig/varnish-cache/lib/libvmod_std/vmod_std.c 2010-11-11 16:42:11.000000000 +0530
+++ varnish/varnish-cache/lib/libvmod_std/vmod_std.c 2010-11-11 18:36:17.000000000 +0530
@@ -27,10 +27,18 @@
*/
#include <ctype.h>
+#include <fcntl.h>
+#include <pthread.h>
#include <stdarg.h>
+#include <stdio.h>
#include <stdlib.h>
#include <syslog.h>
+#include <unistd.h>
#include <netinet/in.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
#include "vrt.h"
#include "../../bin/varnishd/cache.h"
@@ -161,3 +169,127 @@
if (p != NULL)
syslog(fac, "%s", buf);
}
+
+VSLIST_HEAD(cached_file_list, cached_file);
+
+struct cached_file {
+ unsigned magic;
+#define CACHED_FILE_MAGIC 0xa8e9d87a
+ char *file_name;
+ char *contents;
+ time_t last_modification;
+ off_t file_sz;
+ VSLIST_ENTRY(cached_file) next;
+};
+
+static void
+free_cached_files(void *file_list)
+{
+ struct cached_file *iter, *tmp;
+ struct cached_file_list *list = file_list;
+ VSLIST_FOREACH_SAFE(iter, list, next, tmp) {
+ CHECK_OBJ(iter, CACHED_FILE_MAGIC);
+ free(iter->file_name);
+ munmap(iter->contents, iter->file_sz);
+ FREE_OBJ(iter);
+ }
+ free(file_list);
+}
+
+pthread_rwlock_t filelist_lock = PTHREAD_RWLOCK_INITIALIZER;
+int update_counter = 0;
+
+const char *
+vmod_fileread(struct sess *sp, struct vmod_priv *priv, const char *file_name)
+{
+ struct cached_file *iter = NULL;
+ struct stat buf;
+ struct cached_file_list *list;
+ int fd, aligned_sz, pagesize, my_update_counter;
+
+ pagesize = getpagesize();
+
+ (void)sp;
+
+ AZ(pthread_rwlock_rdlock(&filelist_lock));
+
+ if (priv->free == NULL) {
+ AZ(pthread_rwlock_unlock(&filelist_lock));
+ /*
+ * Another thread may already have initialized priv
+ * here, making the repeat check necessary.
+ */
+ AZ(pthread_rwlock_wrlock(&filelist_lock));
+ if (priv->free == NULL) {
+ priv->free = free_cached_files;
+ priv->priv = malloc(sizeof(struct cached_file_list));
+ list = priv->priv;
+ VSLIST_INIT(list);
+ }
+ AZ(pthread_rwlock_unlock(&filelist_lock));
+ AZ(pthread_rwlock_rdlock(&filelist_lock));
+ } else {
+ list = priv->priv;
+ VSLIST_FOREACH(iter, list, next) {
+ CHECK_OBJ(iter, CACHED_FILE_MAGIC);
+ if (strcmp(iter->file_name, file_name) == 0) {
+ /* This thread was holding a read lock. */
+ AZ(pthread_rwlock_unlock(&filelist_lock));
+ return iter->contents;
+ }
+ }
+ }
+
+ my_update_counter = update_counter;
+
+ /* This thread was holding a read lock. */
+ pthread_rwlock_unlock(&filelist_lock);
+
+ if ((fd = open(file_name, O_RDONLY)) == -1)
+ return "";
+
+ fstat(fd, &buf);
+
+ if (buf.st_size % pagesize == 0) {
+ /* XXX: This will lead to a seg-fault. */
+ close(fd);
+ return "";
+ }
+
+ AZ(pthread_rwlock_wrlock(&filelist_lock));
+
+ if (my_update_counter != update_counter) {
+
+ /*
+ * Small optimization: search through the linked list again
+ * only if something has been changed.
+ */
+ VSLIST_FOREACH(iter, list, next) {
+ CHECK_OBJ(iter, CACHED_FILE_MAGIC);
+ if (strcmp(iter->file_name, file_name) == 0) {
+ /* This thread was holding a write lock. */
+ AZ(pthread_rwlock_unlock(&filelist_lock));
+ return iter->contents;
+ }
+ }
+ }
+
+ ALLOC_OBJ(iter, CACHED_FILE_MAGIC);
+
+ iter->file_name = strdup(file_name);
+ iter->last_modification = buf.st_mtime;
+
+ iter->contents = mmap(NULL, buf.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ assert(iter->contents != (void *) -1);
+ close(fd);
+
+ iter->file_sz = buf.st_size;
+
+ assert (iter->contents [iter->file_sz] == 0);
+
+ VSLIST_INSERT_HEAD(list, iter, next);
+
+ my_update_counter++;
+ AZ(pthread_rwlock_unlock(&filelist_lock));
+ return iter->contents;
+}
_______________________________________________
varnish-dev mailing list
[email protected]
http://lists.varnish-cache.org/mailman/listinfo/varnish-dev
