From 50a319df2bdec7cf0056cbbe7230818f86e58751 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Mon, 9 Jan 2023 23:15:08 +0100 Subject: [PATCH] [httpd] Try doing request handling in worker thread --- src/httpd.c | 102 ++--------------------- src/httpd_daap.c | 25 +++--- src/httpd_internal.h | 22 ++--- src/httpd_libevhttp.c | 187 ++++++++++++++++++++++++++++++++++++++---- src/httpd_streaming.c | 21 ----- 5 files changed, 205 insertions(+), 152 deletions(-) diff --git a/src/httpd.c b/src/httpd.c index 970b1b03..5694b997 100644 --- a/src/httpd.c +++ b/src/httpd.c @@ -291,65 +291,12 @@ modules_search(const char *path) /* --------------------------- REQUEST HELPERS ------------------------------ */ -static void -request_unset(struct httpd_request *hreq) +void +httpd_request_handler_set(struct httpd_request *hreq) { - if (hreq->out_body) - evbuffer_free(hreq->out_body); - - httpd_uri_parsed_free(hreq->uri_parsed); - httpd_backend_data_free(hreq->backend_data); -} - -static void -request_set(struct httpd_request *hreq, httpd_backend *backend, const char *uri, const char *user_agent) -{ - httpd_backend_data *backend_data; struct httpd_uri_map *map; int ret; - memset(hreq, 0, sizeof(struct httpd_request)); - - // Populate hreq by getting values from the backend (or from the caller) - hreq->backend = backend; - if (backend) - { - backend_data = httpd_backend_data_create(backend); - hreq->backend_data = backend_data; - - hreq->uri = httpd_backend_uri_get(backend, backend_data); - hreq->uri_parsed = httpd_uri_parsed_create(backend); - - hreq->in_headers = httpd_backend_input_headers_get(backend); - hreq->out_headers = httpd_backend_output_headers_get(backend); - hreq->in_body = httpd_backend_input_buffer_get(backend); - httpd_backend_method_get(&hreq->method, backend); - httpd_backend_peer_get(&hreq->peer_address, &hreq->peer_port, backend, backend_data); - - hreq->user_agent = httpd_header_find(hreq->in_headers, "User-Agent"); - } - else - { - hreq->uri = uri; - hreq->uri_parsed = httpd_uri_parsed_create_fromuri(uri); - - hreq->user_agent = user_agent; - } - - if (!hreq->uri_parsed) - { - DPRINTF(E_LOG, L_HTTPD, "Unable to parse URI '%s' in request from '%s'\n", hreq->uri, hreq->peer_address); - return; - } - - // Don't write directly to backend's buffer. This way we are sure we own the - // buffer even if there is no backend. - CHECK_NULL(L_HTTPD, hreq->out_body = evbuffer_new()); - - hreq->path = httpd_uri_path_get(hreq->uri_parsed); - hreq->query = httpd_uri_query_get(hreq->uri_parsed); - httpd_uri_path_parts_get(&hreq->path_parts, hreq->uri_parsed); - // Path with e.g. /api -> JSON module hreq->module = modules_search(hreq->path); if (!hreq->module) @@ -855,36 +802,28 @@ handle_cors_preflight(struct httpd_request *hreq, const char *allow_origin) } static void -httpd_gen_cb(httpd_backend *backend, void *arg) +request_cb(struct httpd_request *hreq, void *arg) { - struct httpd_request hreq_stack; - struct httpd_request *hreq = &hreq_stack; // Shorthand - - // This is to make modifications to e.g. evhttps's request object - httpd_backend_preprocess(backend); - - // Populates the hreq struct - request_set(hreq, backend, NULL, NULL); - // Did we get a CORS preflight request? if (handle_cors_preflight(hreq, httpd_allow_origin) == 0) { - goto out; + return; } if (!hreq->uri || !hreq->uri_parsed) { DPRINTF(E_WARN, L_HTTPD, "Invalid URI in request: '%s'\n", hreq->uri); httpd_redirect_to(hreq, "/"); - goto out; + return; } else if (!hreq->path) { DPRINTF(E_WARN, L_HTTPD, "Invalid path in request: '%s'\n", hreq->uri); httpd_redirect_to(hreq, "/"); - goto out; + return; } + httpd_request_handler_set(hreq); if (hreq->module) { DPRINTF(E_DBG, hreq->module->logdomain, "%s request: '%s' (thread %ld)\n", hreq->module->name, hreq->uri, syscall(SYS_gettid)); @@ -896,26 +835,11 @@ httpd_gen_cb(httpd_backend *backend, void *arg) DPRINTF(E_DBG, L_HTTPD, "HTTP request: '%s'\n", hreq->uri); serve_file(hreq); } - - out: - request_unset(hreq); } /* ------------------------------- HTTPD API -------------------------------- */ -void -httpd_request_unset(struct httpd_request *hreq) -{ - request_unset(hreq); -} - -void -httpd_request_set(struct httpd_request *hreq, const char *uri, const char *user_agent) -{ - request_set(hreq, NULL, uri, user_agent); -} - /* Thread: httpd */ void httpd_stream_file(struct httpd_request *hreq, int id) @@ -1484,16 +1408,6 @@ httpd_basic_auth(struct httpd_request *hreq, const char *user, const char *passw return -1; } -void -httpd_peer_get(const char **address, ev_uint16_t *port, struct evhttp_connection *evcon) -{ -#ifdef HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR - evhttp_connection_get_peer(evcon, address, port); -#else - evhttp_connection_get_peer(evcon, (char **)address, port); -#endif -} - /* Thread: main */ int httpd_init(const char *webroot) @@ -1537,7 +1451,7 @@ httpd_init(const char *webroot) event_add(exitev, NULL); httpd_port = cfg_getint(cfg_getsec(cfg, "library"), "port"); - httpd_serv = httpd_server_new(evbase_httpd, httpd_port, httpd_gen_cb, NULL); + httpd_serv = httpd_server_new(evbase_httpd, httpd_port, request_cb, NULL); if (!httpd_serv) { DPRINTF(E_FATAL, L_HTTPD, "Could not create HTTP server on port %d (server already running?)\n", httpd_port); diff --git a/src/httpd_daap.c b/src/httpd_daap.c index 640a5bd9..7dca5f94 100644 --- a/src/httpd_daap.c +++ b/src/httpd_daap.c @@ -2306,17 +2306,22 @@ daap_session_is_valid(int id) struct evbuffer * daap_reply_build(const char *uri, const char *user_agent, int is_remote) { - struct httpd_request hreq; - struct evbuffer *out_body; + struct httpd_request *hreq; + struct evbuffer *out_body = NULL; struct daap_session session; int ret; DPRINTF(E_DBG, L_DAAP, "Building reply for DAAP request: '%s'\n", uri); - out_body = NULL; + hreq = httpd_request_new(NULL, uri, user_agent); + if (!hreq) + { + DPRINTF(E_LOG, L_DAAP, "Error building request: '%s'\n", uri); + goto out; + } - httpd_request_set(&hreq, uri, user_agent); - if (!(&hreq)->handler) + httpd_request_handler_set(hreq); + if (!hreq->handler) { DPRINTF(E_LOG, L_DAAP, "Cannot build reply, unrecognized path in request: '%s'\n", uri); goto out; @@ -2325,20 +2330,20 @@ daap_reply_build(const char *uri, const char *user_agent, int is_remote) memset(&session, 0, sizeof(struct daap_session)); session.is_remote = (bool)is_remote; - hreq.extra_data = &session; + hreq->extra_data = &session; - ret = hreq.handler(&hreq); + ret = hreq->handler(hreq); if (ret < 0) { goto out; } // Take ownership of the reply - out_body = hreq.out_body; - hreq.out_body = NULL; + out_body = hreq->out_body; + hreq->out_body = NULL; out: - httpd_request_unset(&hreq); + httpd_request_free(hreq); return out_body; } diff --git a/src/httpd_internal.h b/src/httpd_internal.h index bab5e07f..76c4bce6 100644 --- a/src/httpd_internal.h +++ b/src/httpd_internal.h @@ -41,13 +41,13 @@ struct httpd_request; // Declaring here instead of including event2/http.h makes it easier to support // other backends than evhttp in the future, e.g. libevhtp -struct evhttp; +struct httpd_server; struct evhttp_connection; struct evhttp_request; struct evkeyvalq; struct httpd_uri_parsed; -typedef struct evhttp httpd_server; +typedef struct httpd_server httpd_server; typedef struct evhttp_connection httpd_connection; typedef struct evhttp_request httpd_backend; typedef struct evkeyvalq httpd_headers; @@ -56,7 +56,7 @@ typedef struct httpd_uri_parsed httpd_uri_parsed; typedef void httpd_backend_data; // Not used for evhttp typedef char *httpd_uri_path_parts[31]; -typedef void (*httpd_general_cb)(httpd_backend *backend, void *arg); +typedef void (*httpd_request_cb)(struct httpd_request *hreq, void *arg); typedef void (*httpd_connection_closecb)(httpd_connection *conn, void *arg); typedef void (*httpd_connection_chunkcb)(httpd_connection *conn, void *arg); typedef void (*httpd_query_iteratecb)(const char *key, const char *val, void *arg); @@ -186,10 +186,7 @@ void httpd_stream_file(struct httpd_request *hreq, int id); void -httpd_request_set(struct httpd_request *hreq, const char *uri, const char *user_agent); - -void -httpd_request_unset(struct httpd_request *hreq); +httpd_request_handler_set(struct httpd_request *hreq); bool httpd_request_not_modified_since(struct httpd_request *hreq, time_t mtime); @@ -290,11 +287,17 @@ httpd_request_closecb_set(struct httpd_request *hreq, httpd_connection_closecb c struct event_base * httpd_request_evbase_get(struct httpd_request *hreq); +void +httpd_request_free(struct httpd_request *hreq); + +struct httpd_request * +httpd_request_new(httpd_backend *backend, const char *uri, const char *user_agent); + void httpd_server_free(httpd_server *server); httpd_server * -httpd_server_new(struct event_base *evbase, unsigned short port, httpd_general_cb cb, void *arg); +httpd_server_new(struct event_base *evbase, unsigned short port, httpd_request_cb cb, void *arg); void httpd_server_allow_origin_set(httpd_server *server, bool allow); @@ -344,9 +347,6 @@ httpd_backend_peer_get(const char **addr, uint16_t *port, httpd_backend *backend int httpd_backend_method_get(enum httpd_methods *method, httpd_backend *backend); -void -httpd_backend_preprocess(httpd_backend *backend); - httpd_uri_parsed * httpd_uri_parsed_create(httpd_backend *backend); diff --git a/src/httpd_libevhttp.c b/src/httpd_libevhttp.c index c948d7f0..49eac9f0 100644 --- a/src/httpd_libevhttp.c +++ b/src/httpd_libevhttp.c @@ -1,14 +1,41 @@ +/* + * Copyright (C) 2023 Espen Jürgensen + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifdef HAVE_CONFIG_H +# include +#endif + #include #include #include #include #include +#include #include +#include #include "misc.h" // For net_evhttp_bind +#include "worker.h" +#include "logger.h" #include "httpd_internal.h" + struct httpd_uri_parsed { struct evhttp_uri *ev_uri; @@ -17,6 +44,19 @@ struct httpd_uri_parsed httpd_uri_path_parts path_parts; }; +struct httpd_server +{ + struct evhttp *evhttp; + httpd_request_cb request_cb; + void *request_cb_arg; +}; + +struct cmdargs +{ + httpd_server *server; + httpd_backend *backend; +}; + const char * httpd_query_value_find(httpd_query *query, const char *key) @@ -107,29 +147,148 @@ httpd_request_evbase_get(struct httpd_request *hreq) return evhttp_connection_get_base(conn); } +void +httpd_request_free(struct httpd_request *hreq) +{ + if (!hreq) + return; + + if (hreq->out_body) + evbuffer_free(hreq->out_body); + + httpd_uri_parsed_free(hreq->uri_parsed); + httpd_backend_data_free(hreq->backend_data); + free(hreq); +} + +struct httpd_request * +httpd_request_new(httpd_backend *backend, const char *uri, const char *user_agent) +{ + struct httpd_request *hreq; + httpd_backend_data *backend_data; + + CHECK_NULL(L_HTTPD, hreq = calloc(1, sizeof(struct httpd_request))); + + // Populate hreq by getting values from the backend (or from the caller) + hreq->backend = backend; + if (backend) + { + backend_data = httpd_backend_data_create(backend); + hreq->backend_data = backend_data; + + hreq->uri = httpd_backend_uri_get(backend, backend_data); + hreq->uri_parsed = httpd_uri_parsed_create(backend); + + hreq->in_headers = httpd_backend_input_headers_get(backend); + hreq->out_headers = httpd_backend_output_headers_get(backend); + hreq->in_body = httpd_backend_input_buffer_get(backend); + httpd_backend_method_get(&hreq->method, backend); + httpd_backend_peer_get(&hreq->peer_address, &hreq->peer_port, backend, backend_data); + + hreq->user_agent = httpd_header_find(hreq->in_headers, "User-Agent"); + } + else + { + hreq->uri = uri; + hreq->uri_parsed = httpd_uri_parsed_create_fromuri(uri); + + hreq->user_agent = user_agent; + } + + if (!hreq->uri_parsed) + { + DPRINTF(E_LOG, L_HTTPD, "Unable to parse URI '%s' in request from '%s'\n", hreq->uri, hreq->peer_address); + goto error; + } + + // Don't write directly to backend's buffer. This way we are sure we own the + // buffer even if there is no backend. + CHECK_NULL(L_HTTPD, hreq->out_body = evbuffer_new()); + + hreq->path = httpd_uri_path_get(hreq->uri_parsed); + hreq->query = httpd_uri_query_get(hreq->uri_parsed); + httpd_uri_path_parts_get(&hreq->path_parts, hreq->uri_parsed); + + return hreq; + + error: + httpd_request_free(hreq); + return NULL; +} + +static void +request_free_cb(httpd_backend *backend, void *arg) +{ + struct httpd_request *hreq = arg; + + httpd_request_free(hreq); +} + +// Executed in a worker thread +static void +gencb_worker_cb(void *arg) +{ + struct cmdargs *cmd = arg; + httpd_server *server = cmd->server; + httpd_backend *backend = cmd->backend; + struct httpd_request *hreq; + + hreq = httpd_request_new(backend, NULL, NULL); + if (!hreq) + { + evhttp_send_error(backend, HTTP_INTERNAL, "Internal error"); + return; + } + + evhttp_request_set_on_complete_cb(backend, request_free_cb, hreq); + + server->request_cb(hreq, server->request_cb_arg); +} + +// Callback from evhttp in httpd thread +static void +gencb_httpd(httpd_backend *backend, void *server) +{ + struct cmdargs cmd; + + cmd.server = server; + cmd.backend = backend; + + // Clear the proxy request flag set by evhttp if the request URI was absolute. + // It has side-effects on Connection: keep-alive + backend->flags &= ~EVHTTP_PROXY_REQUEST; + + // Defer the execution to a worker thread + worker_execute(gencb_worker_cb, &cmd, sizeof(cmd), 0); +} + void httpd_server_free(httpd_server *server) { if (!server) return; - evhttp_free(server); + evhttp_free(server->evhttp); + free(server); } httpd_server * -httpd_server_new(struct event_base *evbase, unsigned short port, httpd_general_cb cb, void *arg) +httpd_server_new(struct event_base *evbase, unsigned short port, httpd_request_cb cb, void *arg) { + httpd_server *server; int ret; - struct evhttp *server = evhttp_new(evbase); - if (!server) - goto error; + CHECK_NULL(L_HTTPD, server = calloc(1, sizeof(httpd_server))); + CHECK_NULL(L_HTTPD, server->evhttp = evhttp_new(evbase)); - ret = net_evhttp_bind(server, port, "httpd"); + server->request_cb = cb; + server->request_cb_arg = arg; + + ret = net_evhttp_bind(server->evhttp, port, "httpd"); if (ret < 0) goto error; - evhttp_set_gencb(server, cb, arg); + evhttp_set_gencb(server->evhttp, gencb_httpd, server); return server; @@ -141,7 +300,7 @@ httpd_server_new(struct event_base *evbase, unsigned short port, httpd_general_c void httpd_server_allow_origin_set(httpd_server *server, bool allow) { - evhttp_set_allowed_methods(server, EVHTTP_REQ_GET | EVHTTP_REQ_POST | EVHTTP_REQ_PUT | EVHTTP_REQ_DELETE | EVHTTP_REQ_HEAD | EVHTTP_REQ_OPTIONS); + evhttp_set_allowed_methods(server->evhttp, EVHTTP_REQ_GET | EVHTTP_REQ_POST | EVHTTP_REQ_PUT | EVHTTP_REQ_DELETE | EVHTTP_REQ_HEAD | EVHTTP_REQ_OPTIONS); } void @@ -223,7 +382,11 @@ httpd_backend_peer_get(const char **addr, uint16_t *port, httpd_backend *backend if (!conn) return -1; +#ifdef HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR + evhttp_connection_get_peer(conn, addr, port); +#else evhttp_connection_get_peer(conn, (char **)addr, port); +#endif return 0; } @@ -249,14 +412,6 @@ httpd_backend_method_get(enum httpd_methods *method, httpd_backend *backend) return 0; } -void -httpd_backend_preprocess(httpd_backend *backend) -{ - // Clear the proxy request flag set by evhttp if the request URI was absolute. - // It has side-effects on Connection: keep-alive - backend->flags &= ~EVHTTP_PROXY_REQUEST; -} - httpd_uri_parsed * httpd_uri_parsed_create(httpd_backend *backend) { diff --git a/src/httpd_streaming.c b/src/httpd_streaming.c index 85a82f41..65994c49 100644 --- a/src/httpd_streaming.c +++ b/src/httpd_streaming.c @@ -162,35 +162,14 @@ static void streaming_end(void) { struct streaming_session *session; -<<<<<<< HEAD - struct evhttp_connection *evcon; - const char *address; - ev_uint16_t port; -======= ->>>>>>> [httpd] Remove all traces of evhttp from httpd modules pthread_mutex_lock(&streaming_sessions_lck); for (session = streaming_sessions; streaming_sessions; session = streaming_sessions) { -<<<<<<< HEAD - evcon = evhttp_request_get_connection(session->req); - if (evcon) - { - evhttp_connection_set_closecb(evcon, NULL, NULL); - httpd_peer_get(&address, &port, evcon); - DPRINTF(E_INFO, L_STREAMING, "Force close stream to %s:%d\n", address, (int)port); - } - evhttp_send_reply_end(session->req); -======= DPRINTF(E_INFO, L_STREAMING, "Force close stream to %s:%d\n", session->hreq->peer_address, (int)session->hreq->peer_port); httpd_request_closecb_set(session->hreq, NULL, NULL); -<<<<<<< HEAD - httpd_reply_end_send(session->hreq); ->>>>>>> [httpd] Remove all traces of evhttp from httpd modules -======= httpd_send_reply_end(session->hreq); ->>>>>>> [httpd] Changes to httpd_send_reply_chunk et al streaming_sessions = session->next; free(session);