* fix Makefile to build in a (more) sensible way (still not really perfect)
authorTero Marttila <terom@fixme.fi>
Fri, 06 Jun 2008 03:24:22 +0300
changeset 8 4d38ccbeb93e
parent 7 446c0b816b91
child 9 fb6632e6c1bb
* fix Makefile to build in a (more) sensible way (still not really perfect)
* a parse_hostport in common
* fix warnings in render_file
* commandline argument parsing for render_node
* render_remote takes a struct remote_node, maintains its current_load, and actually close the socket after use now
* web_main uses a remote_pool, accepts nodes on the command line, and picks nodes from there for render_remote
* improve _http_render_execute error handling, sends back an error reply now
* ignore SIGPIPE. This hadn't shown up before (probably read-EOF instead), but it uncovered a bug in evhttp that caused an infinite bufferevent_write loop -> oom_killer -> DoS attack
* yes, this commit is missing four new files, those will be included in the next one

committer: Tero Marttila <terom@fixme.fi>
Makefile
common.c
common.h
render_file.c
render_node.c
render_remote.c
render_remote.h
web_main.c
--- a/Makefile	Thu Jun 05 23:04:28 2008 +0300
+++ b/Makefile	Fri Jun 06 03:24:22 2008 +0300
@@ -1,19 +1,24 @@
 LDFLAGS = -Llib/libevent-dev/lib -levent -lpng
 CFLAGS = -Wall -g -Ilib/libevent-dev/include
 
-OBJS = common.o mandelbrot.o render.o render_remote.o
-HEADERS = common.h mandelbrot.h render.h
 EXECS = render_file web_main render_node
 
-all: ${EXECS}
+all: render_file render_node web_main
 
-render_file: ${OBJS}
+common.o: common.c common.h
+remote_node.o: remote_node.c remote_node.h
+remote_pool.o: remote_pool.c remote_pool.h
+render.o: render.c render.h
+render_remote.o: render_remote.c render_remote.h
 
-web_main: ${OBJS}
+render_file.o: render_file.c
+render_node.o: render_node.c
+web_main.o: web_main.c
 
-render_node: ${OBJS}
+render_file: render_file.o common.o render.o mandelbrot.o
+render_node: render_node.o common.o render.o mandelbrot.o
+web_main: web_main.o common.o render.o remote_node.o remote_pool.o render_remote.o
 
 clean :
-	rm ${OBJS}
-	rm ${EXECS}
+	rm *.o ${EXECS}
 
--- a/common.c	Thu Jun 05 23:04:28 2008 +0300
+++ b/common.c	Fri Jun 06 03:24:22 2008 +0300
@@ -3,6 +3,7 @@
 #include <stdarg.h>
 #include <errno.h>
 #include <string.h>
+#include <ctype.h>
 
 #include "common.h"
 
@@ -52,3 +53,98 @@
     fprintf(stderr, "\n");
     exit(EXIT_FAILURE);
 }
+
+int parse_hostport (char *hostport, char **host, char **port) {
+    char *c;
+
+    char brace = 0, colon = 0;
+
+    *host = *port = NULL;
+
+    for (c = hostport; *c != '\0'; c++) {
+        if (isspace(*c))
+            continue;
+
+        switch (*c) {
+            case '[':
+                if (c > hostport) {
+                    error("parse_hostport: %c must be first char, if any: %s", *c, hostport);
+                    return -1;
+                }
+                
+                brace = *c;
+                break;
+
+            case ']':
+                if (!brace) {
+                    error("parse_hostport: %c without matching brace: %s", *c, hostport);
+                    return -1;
+                }
+
+                if (!*host) {
+                    error("parse_hostport: empty hostname: %s", hostport);
+                    return -1;
+                }
+
+                brace = 0;
+                break;
+            
+            case ':':
+                if (brace) {
+                    // colons inside braces are left as-is
+                    continue;
+                }
+
+                if (!*host) {
+                    error("parse_hostport: colon before hostname: %s", hostport);
+                    return -1;
+                }
+
+                if (*port) {
+                    error("parse_hostport: colon after port: %s", hostport);
+                    return -1;
+                }
+
+                if (colon) {
+                    error("parse_hostport: too many colons: %s", hostport);
+                    return -1;
+                };
+
+                // finished parsing the host part, move on to the port part
+                colon = ':';
+                *c = '\0';
+                break;
+
+            default:
+                if (!*host) {
+                    // first char of the host
+                    *host = c;
+
+                } else if (colon && !*port) {
+                    // first char of the port
+                    *port = c;
+
+                } // else a part of either, don't care about it
+
+                break;
+        }
+    }
+
+    if (!*host) {
+        error("parse_hostport: missing hostname: %s", hostport);
+        return -1;
+    }
+
+    if (brace) {
+        error("parse_hostport: missing close-brace for %c: %s", brace, hostport);
+        return -1;
+    }
+
+    if (colon && !*port) {
+        error("parse_hostport: missing port: %s", hostport);
+        return -1;
+    }
+
+    return 0;
+}
+
--- a/common.h	Thu Jun 05 23:04:28 2008 +0300
+++ b/common.h	Fri Jun 06 03:24:22 2008 +0300
@@ -1,7 +1,38 @@
+
+/*
+ * error handling
+ */
+
+// perror + exit
 void die (const char *msg);
+
+// fprintf + newline
 void error (const char *fmt, ...);
+
+// fprintf + strerror + newline
 void perr (const char *fmt, ...);
+
+// fprintf + strerror + newline + exit
 void perr_exit (const char *fmt, ...);
+
+// fprintf + newline + exit
 void err_exit (const char *fmt, ...);
 
+/*
+ * Parse a host:port string.
+ *
+ * Valid formats:
+ *  host
+ *  host:port
+ *  [host]
+ *  [host]:port
+ *
+ * The contents of the given hostport string *will* be modified.
+ *
+ * The value of *port will be set to NULL if no port was given.
+ *
+ * Returns 0 and sets *host if succesfull, nonzero otherwise.
+ *
+ */
+int parse_hostport (char *hostport, char **host, char **port);
 
--- a/render_file.c	Thu Jun 05 23:04:28 2008 +0300
+++ b/render_file.c	Fri Jun 06 03:24:22 2008 +0300
@@ -6,6 +6,7 @@
 #include <sys/socket.h>
 #include <arpa/inet.h>
 
+#include "render.h"
 #include "mandelbrot.h"
 #include "common.h"
 
@@ -107,7 +108,7 @@
 int main (int argc, char **argv) {
     int opt;
     
-    FILE *output = NULL, *remote = NULL;
+    FILE *output = NULL /*, *remote = NULL */ ;
     int img_w = 256, img_h = 256;
     
 
--- a/render_node.c	Thu Jun 05 23:04:28 2008 +0300
+++ b/render_node.c	Fri Jun 06 03:24:22 2008 +0300
@@ -5,6 +5,7 @@
 #include <stdlib.h>
 #include <signal.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "render.h"
 #include "render_remote.h"
@@ -115,19 +116,41 @@
     int ssock, sock;
     struct sockaddr_in addr;
     socklen_t addr_len;
+    
+    // parse arguments
+    int opt;
+    const char *port_name = NULL;
 
+    while ((opt = getopt(argc, argv, "l:")) != -1) {
+        switch (opt) {
+            case 'l':
+                if (port_name)
+                    err_exit("only specify -l once");
+
+                port_name = optarg;
+                break;
+
+            default:
+                err_exit("Usage: %s [-l port]", argv[0]);
+        }
+    }
+
+    if (!port_name)
+        port_name = RENDER_PORT_NAME;
+
+    unsigned short port = atoi(port_name);
+
+    if (!port)
+        err_exit("invalid port: %s", port_name);
 
     // create the socket
     if ((ssock = socket(PF_INET, SOCK_STREAM, 0)) == -1)
         perr_exit("socket");
 
     addr.sin_family = AF_INET;
-    addr.sin_port = htons(RENDER_PORT);
+    addr.sin_port = htons(port);
     addr.sin_addr.s_addr = INADDR_ANY;
 
-    if (argc > 1)
-        addr.sin_port = htons(atoi(argv[1]));
-    
     if (bind(ssock, (struct sockaddr *) &addr, sizeof(struct sockaddr_in)) == -1)
         perr_exit("bind");
     
--- a/render_remote.c	Thu Jun 05 23:04:28 2008 +0300
+++ b/render_remote.c	Fri Jun 06 03:24:22 2008 +0300
@@ -14,6 +14,8 @@
     struct event *ev_conn;
     struct bufferevent *data_bev;
 
+    struct remote_node *remote_node;
+
     #pragma pack(push)
     #pragma pack(1)
 
@@ -39,27 +41,33 @@
     void *cb_arg;
 };
 
-void _remote_render_ctx_free (struct remote_render_ctx **ctx) {
+void _remote_render_ctx_free (struct remote_render_ctx *ctx) {
+    // close the socket (ctx->ev_conn remains valid even after we're done with it...)
+    close(event_get_fd(ctx->ev_conn));
+    
+    // free the connect event
+    event_free(ctx->ev_conn);
+
     // free the data_bev
-    if ((*ctx)->data_bev) {
-        bufferevent_free((*ctx)->data_bev);
-        (*ctx)->data_bev = NULL;
+    if (ctx->data_bev) {
+        bufferevent_free(ctx->data_bev);
+        ctx->data_bev = NULL;
     }
 
-    // and the event
-    event_free((*ctx)->ev_conn);
+    // update remote_node load
+    ctx->remote_node->current_load--;
     
     // free the context structure
-    free(*ctx);
+    free(ctx);
     
-    *ctx = NULL;
+    ctx = NULL;
 }
 
 #define RENDER_FAILED(ctx, desc) \
     do {                                        \
         perror(desc);                           \
         ctx->cb_fail(ctx->cb_arg);              \
-        _remote_render_ctx_free(&ctx);          \
+        _remote_render_ctx_free(ctx);          \
         return;                                 \
     } while (0)
 
@@ -117,7 +125,7 @@
     }
 
     // free resources
-    _remote_render_ctx_free(&ctx);
+    _remote_render_ctx_free(ctx);
 }
 
 void _remote_connected (int fd, short event, void *arg) {
@@ -149,13 +157,15 @@
 
 struct remote_render_ctx *render_remote (
         render_t *render_ctx,
-        struct sockaddr_storage *remote,
+        struct remote_node *remote_node,
         void (*cb_sent)(void *arg),
         void (*cb_data)(struct evbuffer *buf, void *arg),
         void (*cb_done)(void *arg),
         void (*cb_fail)(void *arg),
         void *cb_arg
 ) {    
+    printf("render_remote: remote render load: %d/%d\n", remote_node->current_load, remote_node->parallel_renders);
+
     // alloc the remote render ctx
     struct remote_render_ctx *ctx = malloc(sizeof(struct remote_render_ctx));
 
@@ -170,12 +180,15 @@
     ctx->cb_done = cb_done;
     ctx->cb_fail = cb_fail;
     ctx->cb_arg = cb_arg;
+
+    // keep a reference to remote_node so we can decr it's load
+    ctx->remote_node = remote_node;
     
     // copy the relevant stuff from the render_ctx
     render_cmd_build(render_ctx, ctx);
     
     // create the socket
-    int sock = socket(remote->ss_family, SOCK_STREAM, 0);
+    int sock = socket(remote_node->addr.ss_family, SOCK_STREAM, 0);
 
     if (sock < 0) {
         perror("render_remote: socket");
@@ -189,7 +202,7 @@
     }
     
     // initiate the connect
-    int err = connect(sock, (struct sockaddr *) remote, sizeof(*remote));
+    int err = connect(sock, (struct sockaddr *) &remote_node->addr, sizeof(remote_node->addr));
 
     if (err != -1 || errno != EINPROGRESS) {
         perror("render_remote: connect");
@@ -208,6 +221,9 @@
         error("render_remote: event_add");
         goto error;
     }
+
+    // update remote_node load
+    remote_node->current_load++;
     
     // success
     return ctx;
@@ -243,13 +259,9 @@
     // if it's still just connecting, cancel that
     if (event_pending(ctx->ev_conn, EV_WRITE, NULL)) {
         event_del(ctx->ev_conn);
-
     }
     
-    // close the socket (ctx->ev_conn remains valid even after we're done with it...)
-    close(event_get_fd(ctx->ev_conn));
-    
     // this takes care of the rest
-    _remote_render_ctx_free (&ctx);
+    _remote_render_ctx_free (ctx);
 }
 
--- a/render_remote.h	Thu Jun 05 23:04:28 2008 +0300
+++ b/render_remote.h	Fri Jun 06 03:24:22 2008 +0300
@@ -5,13 +5,14 @@
 #include <event2/buffer.h>
 
 #include "render.h"
+#include "remote_node.h"
 
 /*
  * Execute a render_t operation on a remote render_node
  */
 
 
-#define RENDER_PORT 6159
+#define RENDER_PORT_NAME "6159"
 
 struct remote_render_ctx;
 
@@ -27,12 +28,12 @@
  */
 struct remote_render_ctx *render_remote (
         render_t *render_ctx,               // what to render
-        struct sockaddr_storage *remote,    // what render node to use
+        struct remote_node *remote_node,    // what render node to use
         void (*cb_sent)(void *arg),
         void (*cb_data)(struct evbuffer *buf, void *arg),
         void (*cb_done)(void *arg),
         void (*cb_fail)(void *arg),
-        void *cb_arg;
+        void *cb_arg
 );
 
 /*
--- a/web_main.c	Thu Jun 05 23:04:28 2008 +0300
+++ b/web_main.c	Fri Jun 06 03:24:22 2008 +0300
@@ -16,6 +16,8 @@
 
 #include "render.h"
 #include "render_remote.h"
+#include "remote_node.h"
+#include "remote_pool.h"
 #include "common.h"
 
 #define MIN_CHUNK_SIZE 4096
@@ -27,8 +29,8 @@
 // what event_base we're using
 static struct event_base *ev_base;
 
-// what render node to use
-static struct sockaddr_storage render_node;
+// our render node pool
+static struct remote_pool remote_pool;
 
 // info on a render request
 struct render_request {
@@ -160,8 +162,16 @@
 }
 
 void _http_render_execute (struct evhttp_request *request, u_int32_t img_w, u_int32_t img_h) {
+    // error message
+    const char *errmsg = NULL;
+
+#define ERROR(msg) do { errmsg = msg; goto error; } while (0)
+
     // render request context
     struct render_request *req_ctx = calloc(1, sizeof(struct render_request));
+
+    if (!req_ctx)
+        ERROR("calloc");
     
     req_ctx->http_request = request;
     req_ctx->headers_sent = 0;
@@ -169,27 +179,46 @@
     
     // render context
     render_t rend_ctx;
-    render_init(&rend_ctx, RENDER_PNG);
-    render_set_size(&rend_ctx, img_w, img_h);
-    render_region_full(&rend_ctx);
-    
+    if (render_init(&rend_ctx, RENDER_PNG))
+        ERROR("render_init");
+
+    if (render_set_size(&rend_ctx, img_w, img_h))
+        ERROR("render_set_size");
+
+    if (render_region_full(&rend_ctx))
+        ERROR("render_region_full");
+
+    // pick a render_node
+    struct remote_node *node_info;
+
+    if ((node_info = remote_pool_get(&remote_pool)) == NULL)
+        ERROR("remote_pool_get");
+
     // initiate the remote render operation
-    if ((req_ctx->remote_ctx = render_remote(&rend_ctx, &render_node,
+    if ((req_ctx->remote_ctx = render_remote(&rend_ctx, node_info,
         &_render_sent,
         &_render_data,
         &_render_done,
         &_render_fail,
         req_ctx
-    )) == NULL) {
-        free(req_ctx);
-        fprintf(stderr, "ERR: render_remote\n");
-        return;
-    }
+    )) == NULL)
+        ERROR("render_remote");
+
+#undef ERROR    /* can't be used after request has been sent */
 
     // set close cb
     evhttp_set_reply_abortcb(request, &_render_http_lost, req_ctx);
     
     printf("render [%p]: started\n", req_ctx);
+    
+    return;
+
+error:
+    error("ERR: %s", errmsg);
+    evhttp_send_error(request, 500, "Internal Server Error");
+
+    free(req_ctx);
+        
 }
 
 /*
@@ -254,8 +283,17 @@
 }
 
 void signals_init () {
+    // handle SIGINT
     signal_set(&ev_sigint, SIGINT, &sigint_handler, NULL);
     signal_add(&ev_sigint, NULL);
+    
+    // ignore SIGPIPE
+    struct sigaction sigpipe;
+    memset(&sigpipe, 0, sizeof(sigpipe));
+
+    sigpipe.sa_handler = SIG_IGN;
+
+    sigaction(SIGPIPE, &sigpipe, NULL);
 }
 
 void signals_deinit () {
@@ -273,22 +311,51 @@
     if (!ev_base)
         die("event_init");
     
+    // set up our render node pool
+    remote_pool_init(&remote_pool);
+
+    // process arguments
     int opt;
     int enable_debug = 0;
+    char *host, *port;
 
-    // arguments
-    while ((opt = getopt(argc, argv, "d")) != -1) {
+    while ((opt = getopt(argc, argv, "dp:r:")) != -1) {
         switch (opt) {
+            case 'p':
+                // populate the pool from a file
+                if (remote_pool_load(&remote_pool, optarg))
+                    return 1;
+
+                break;
+
+            case 'r':
+                // add the given render node to the pool
+                if (
+                        parse_hostport(optarg, &host, &port)
+                     || remote_pool_add(&remote_pool, host, port)
+                )
+                    return 1;
+
+                break;
+
             case 'd':
                 // enable libevent debugging
                 enable_debug = 1;
                 break;
 
             default:
-                err_exit("Usage: %s [-d]", argv[0]);
+                err_exit("Usage: %s [-d] (-p pool_file | -r hostname[:port] | ...)", argv[0]);
         
         }
     }
+
+    int pool_size = remote_pool_size(&remote_pool);
+
+    if (!pool_size) {
+        err_exit("No remote render nodes given");
+    }
+
+    printf("Registered %d render nodes in our pool\n", pool_size);
     
     // per default it is enabled
     if (!enable_debug)
@@ -310,13 +377,6 @@
     // add our http request handler
     evhttp_set_cb(http_server, "/render", &http_render, NULL);
 
-    // hack in our render node
-    struct sockaddr_in *render_node_addr = (struct sockaddr_in *) &render_node;
-
-    render_node_addr->sin_family = AF_INET;
-    render_node_addr->sin_port = htons(RENDER_PORT);
-    inet_aton("127.0.0.1", &render_node_addr->sin_addr);
-
     // we shall now run
     printf("RUN 0.0.0.0:8117\n");