View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0007192 | libmicrohttpd | internal event loop | public | 2022-03-11 11:40 | 2024-01-21 13:33 |
Reporter | dro | Assigned To | Christian Grothoff | ||
Priority | normal | Severity | minor | Reproducibility | sometimes |
Status | closed | Resolution | fixed | ||
Platform | arm64 | OS | Debian | OS Version | 9.10 |
Product Version | 0.9.72 | ||||
Target Version | 0.9.76 | Fixed in Version | 0.9.76 | ||
Summary | 0007192: Notify complete callback sometimes not called when connection is closed prematurely while using epoll internal thread | ||||
Description | When using MHD_USE_EPOLL_INTERNAL_THREAD, the callback specified using MHD_OPTION_NOTIFY_COMPLETED is sometimes not called when a connection is closed mid transfer of a large POST request, even though the MHD_AccessHandlerCallback has already been called. I have tested using the versions in the Buster and Bullseye Debian repositories (0.9.62-1 and 0.9.72-2), and the behaviour is the same. When I run the server using MHD_USE_POLL_INTERNAL_THREAD instead, the callback is correctly called every time. | ||||
Steps To Reproduce | Start a libmicrohttpd server using MHD_USE_EPOLL_INTERNAL_THREAD, and define a MHD_OPTION_NOTIFY_COMPLETED callback. MHD_start_daemon(MHD_USE_EPOLL_INTERNAL_THREAD, port, NULL, NULL, &handler, info, MHD_OPTION_THREAD_POOL_SIZE, 8, MHD_OPTION_NOTIFY_COMPLETED, &complete_callback, NULL, MHD_OPTION_NOTIFY_CONNECTION, &connnection_callback, NULL, MHD_OPTION_END); I use curl to post a 1GB file to the server, and at some point in the middle of the transfer, I use ctrl+\ to send a SIGQUIT to curl. The "complete_callback" is only intermittently called in this instance when using EPOLL, but changing to using POLL means that it is called every time. | ||||
Tags | No tags attached. | ||||
|
Do you have a nice (simple) test? Does the problem exist in Git master? |
|
Marco Boss just reported something that might be related (missing notify cleanup) against the same MHD version for the G5k benchmarks. |
|
Sorry for the slow reply. The problem does occur in Git master, and I've attached a minimal example. What I did was checkout Git master, edit the post_example.c example file to be as attached and build it. I then created a 1G test file: dd if=/dev/urandom of=test_1G.dat bs=1M count=1024 Ran the server: ./post_example 12345 In a different terminal performed the curl upload: curl -F 'file=@test_1G.dat' http://127.0.0.1:12345 And while the upload was progressing, sent a SIGQUIT signal to curl by pressing ctrl+\ When the MHD_start_daemon option was set to MHD_USE_EPOLL_INTERNAL_THREAD, as in the example, nothing is printed out to suggest the connection has been closed (ignoring all the "Unsupported form value 'file'" lines). If you edit the post_example.c option to use MHD_USE_POLL_INTERNAL_THREAD instead of MHD_USE_EPOLL_INTERNAL_THREAD and rebuild and run the same test again, when the curl upload is killed, the server prints out "Connection was closed by remote side with incomplete request." and "Connection completed callback called", indicating that the request_completed_callback is being called as expected. post_example.c (22,123 bytes)
/* This file is part of libmicrohttpd Copyright (C) 2011 Christian Grothoff (and other contributing authors) Copyright (C) 2014-2022 Evgeny Grin (Karlson2k) This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version. This library 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 Lesser General Public License for more details. You should have received a copy of the GNU Lesser General Public License along with this library; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ /** * @file post_example.c * @brief example for processing POST requests using libmicrohttpd * @author Christian Grothoff * @author Karlson2k (Evgeny Grin) */ #include <stdlib.h> #include <string.h> #include <stdio.h> #include <errno.h> #include <time.h> #include <microhttpd.h> /** * Invalid method page. */ #define METHOD_ERROR \ "<html><head><title>Illegal request</title></head><body>Go away.</body></html>" /** * Invalid URL page. */ #define NOT_FOUND_ERROR \ "<html><head><title>Not found</title></head><body>Go away.</body></html>" /** * Front page. (/) */ #define MAIN_PAGE \ "<html><head><title>Welcome</title></head><body><form action=\"/2\" method=\"post\">What is your name? <input type=\"text\" name=\"v1\" value=\"%s\" /><input type=\"submit\" value=\"Next\" /></form></body></html>" /** * Second page. (/2) */ #define SECOND_PAGE \ "<html><head><title>Tell me more</title></head><body><a href=\"/\">previous</a> <form action=\"/S\" method=\"post\">%s, what is your job? <input type=\"text\" name=\"v2\" value=\"%s\" /><input type=\"submit\" value=\"Next\" /></form></body></html>" /** * Second page (/S) */ #define SUBMIT_PAGE \ "<html><head><title>Ready to submit?</title></head><body><form action=\"/F\" method=\"post\"><a href=\"/2\">previous </a> <input type=\"hidden\" name=\"DONE\" value=\"yes\" /><input type=\"submit\" value=\"Submit\" /></form></body></html>" /** * Last page. */ #define LAST_PAGE \ "<html><head><title>Thank you</title></head><body>Thank you.</body></html>" /** * Name of our cookie. */ #define COOKIE_NAME "session" /** * State we keep for each user/session/browser. */ struct Session { /** * We keep all sessions in a linked list. */ struct Session *next; /** * Unique ID for this session. */ char sid[33]; /** * Reference counter giving the number of connections * currently using this session. */ unsigned int rc; /** * Time when this session was last active. */ time_t start; /** * String submitted via form. */ char value_1[64]; /** * Another value submitted via form. */ char value_2[64]; }; /** * Data kept per request. */ struct Request { /** * Associated session. */ struct Session *session; /** * Post processor handling form data (IF this is * a POST request). */ struct MHD_PostProcessor *pp; /** * URL to serve in response to this POST (if this request * was a 'POST') */ const char *post_url; }; /** * Linked list of all active sessions. Yes, O(n) but a * hash table would be overkill for a simple example... */ static struct Session *sessions; /** * Return the session handle for this connection, or * create one if this is a new user. */ static struct Session * get_session (struct MHD_Connection *connection) { struct Session *ret; const char *cookie; cookie = MHD_lookup_connection_value (connection, MHD_COOKIE_KIND, COOKIE_NAME); if (cookie != NULL) { /* find existing session */ ret = sessions; while (NULL != ret) { if (0 == strcmp (cookie, ret->sid)) break; ret = ret->next; } if (NULL != ret) { ret->rc++; return ret; } } /* create fresh session */ ret = calloc (1, sizeof (struct Session)); if (NULL == ret) { fprintf (stderr, "calloc error: %s\n", strerror (errno)); return NULL; } /* not a super-secure way to generate a random session ID, but should do for a simple example... */ snprintf (ret->sid, sizeof (ret->sid), "%X%X%X%X", (unsigned int) rand (), (unsigned int) rand (), (unsigned int) rand (), (unsigned int) rand ()); ret->rc++; ret->start = time (NULL); ret->next = sessions; sessions = ret; return ret; } /** * Type of handler that generates a reply. * * @param cls content for the page (handler-specific) * @param mime mime type to use * @param session session information * @param connection connection to process * @param MHD_YES on success, MHD_NO on failure */ typedef enum MHD_Result (*PageHandler)(const void *cls, const char *mime, struct Session *session, struct MHD_Connection *connection); /** * Entry we generate for each page served. */ struct Page { /** * Acceptable URL for this page. */ const char *url; /** * Mime type to set for the page. */ const char *mime; /** * Handler to call to generate response. */ PageHandler handler; /** * Extra argument to handler. */ const void *handler_cls; }; /** * Add header to response to set a session cookie. * * @param session session to use * @param response response to modify */ static void add_session_cookie (struct Session *session, struct MHD_Response *response) { char cstr[256]; snprintf (cstr, sizeof (cstr), "%s=%s", COOKIE_NAME, session->sid); if (MHD_NO == MHD_add_response_header (response, MHD_HTTP_HEADER_SET_COOKIE, cstr)) { fprintf (stderr, "Failed to set session cookie header!\n"); } } /** * Handler that returns a simple static HTTP page that * is passed in via 'cls'. * * @param cls a 'const char *' with the HTML webpage to return * @param mime mime type to use * @param session session handle * @param connection connection to use */ static enum MHD_Result serve_simple_form (const void *cls, const char *mime, struct Session *session, struct MHD_Connection *connection) { enum MHD_Result ret; const char *form = cls; struct MHD_Response *response; /* return static form */ response = MHD_create_response_from_buffer_static (strlen (form), (const void *) form); if (NULL == response) return MHD_NO; add_session_cookie (session, response); MHD_add_response_header (response, MHD_HTTP_HEADER_CONTENT_ENCODING, mime); ret = MHD_queue_response (connection, MHD_HTTP_OK, response); MHD_destroy_response (response); return ret; } /** * Handler that adds the 'v1' value to the given HTML code. * * @param cls unused * @param mime mime type to use * @param session session handle * @param connection connection to use */ static enum MHD_Result fill_v1_form (const void *cls, const char *mime, struct Session *session, struct MHD_Connection *connection) { enum MHD_Result ret; size_t slen; char *reply; struct MHD_Response *response; (void) cls; /* Unused. Silent compiler warning. */ slen = strlen (MAIN_PAGE) + strlen (session->value_1); reply = malloc (slen + 1); if (NULL == reply) return MHD_NO; snprintf (reply, slen + 1, MAIN_PAGE, session->value_1); /* return static form */ response = MHD_create_response_from_buffer_with_free_callback (slen, (void *) reply, &free); if (NULL == response) { free (reply); return MHD_NO; } add_session_cookie (session, response); MHD_add_response_header (response, MHD_HTTP_HEADER_CONTENT_ENCODING, mime); ret = MHD_queue_response (connection, MHD_HTTP_OK, response); MHD_destroy_response (response); return ret; } /** * Handler that adds the 'v1' and 'v2' values to the given HTML code. * * @param cls unused * @param mime mime type to use * @param session session handle * @param connection connection to use */ static enum MHD_Result fill_v1_v2_form (const void *cls, const char *mime, struct Session *session, struct MHD_Connection *connection) { enum MHD_Result ret; char *reply; struct MHD_Response *response; size_t slen; (void) cls; /* Unused. Silent compiler warning. */ slen = strlen (SECOND_PAGE) + strlen (session->value_1) + strlen ( session->value_2); reply = malloc (slen + 1); if (NULL == reply) return MHD_NO; snprintf (reply, slen + 1, SECOND_PAGE, session->value_1, session->value_2); /* return static form */ response = MHD_create_response_from_buffer_with_free_callback (slen, (void *) reply, &free); if (NULL == response) { free (reply); return MHD_NO; } add_session_cookie (session, response); MHD_add_response_header (response, MHD_HTTP_HEADER_CONTENT_ENCODING, mime); ret = MHD_queue_response (connection, MHD_HTTP_OK, response); MHD_destroy_response (response); return ret; } /** * Handler used to generate a 404 reply. * * @param cls a 'const char *' with the HTML webpage to return * @param mime mime type to use * @param session session handle * @param connection connection to use */ static enum MHD_Result not_found_page (const void *cls, const char *mime, struct Session *session, struct MHD_Connection *connection) { enum MHD_Result ret; struct MHD_Response *response; (void) cls; /* Unused. Silent compiler warning. */ (void) session; /* Unused. Silent compiler warning. */ /* unsupported HTTP method */ response = MHD_create_response_from_buffer_static (strlen (NOT_FOUND_ERROR), (const void *) NOT_FOUND_ERROR); if (NULL == response) return MHD_NO; ret = MHD_queue_response (connection, MHD_HTTP_NOT_FOUND, response); MHD_add_response_header (response, MHD_HTTP_HEADER_CONTENT_ENCODING, mime); MHD_destroy_response (response); return ret; } /** * List of all pages served by this HTTP server. */ static struct Page pages[] = { { "/", "text/html", &fill_v1_form, NULL }, { "/2", "text/html", &fill_v1_v2_form, NULL }, { "/S", "text/html", &serve_simple_form, SUBMIT_PAGE }, { "/F", "text/html", &serve_simple_form, LAST_PAGE }, { NULL, NULL, ¬_found_page, NULL } /* 404 */ }; /** * Iterator over key-value pairs where the value * maybe made available in increments and/or may * not be zero-terminated. Used for processing * POST data. * * @param cls user-specified closure * @param kind type of the value * @param key 0-terminated key for the value * @param filename name of the uploaded file, NULL if not known * @param content_type mime-type of the data, NULL if not known * @param transfer_encoding encoding of the data, NULL if not known * @param data pointer to size bytes of data at the * specified offset * @param off offset of data in the overall value * @param size number of bytes in data available * @return MHD_YES to continue iterating, * MHD_NO to abort the iteration */ static enum MHD_Result post_iterator (void *cls, enum MHD_ValueKind kind, const char *key, const char *filename, const char *content_type, const char *transfer_encoding, const char *data, uint64_t off, size_t size) { struct Request *request = cls; struct Session *session = request->session; (void) kind; /* Unused. Silent compiler warning. */ (void) filename; /* Unused. Silent compiler warning. */ (void) content_type; /* Unused. Silent compiler warning. */ (void) transfer_encoding; /* Unused. Silent compiler warning. */ if (0 == strcmp ("DONE", key)) { fprintf (stdout, "Session `%s' submitted `%s', `%s'\n", session->sid, session->value_1, session->value_2); return MHD_YES; } if (0 == strcmp ("v1", key)) { if (size + off >= sizeof(session->value_1)) size = sizeof (session->value_1) - off - 1; memcpy (&session->value_1[off], data, size); session->value_1[size + off] = '\0'; return MHD_YES; } if (0 == strcmp ("v2", key)) { if (size + off >= sizeof(session->value_2)) size = sizeof (session->value_2) - off - 1; memcpy (&session->value_2[off], data, size); session->value_2[size + off] = '\0'; return MHD_YES; } fprintf (stderr, "Unsupported form value `%s'\n", key); return MHD_YES; } /** * Main MHD callback for handling requests. * * @param cls argument given together with the function * pointer when the handler was registered with MHD * @param connection handle identifying the incoming connection * @param url the requested url * @param method the HTTP method used ("GET", "PUT", etc.) * @param version the HTTP version string (i.e. "HTTP/1.1") * @param upload_data the data being uploaded (excluding HEADERS, * for a POST that fits into memory and that is encoded * with a supported encoding, the POST data will NOT be * given in upload_data and is instead available as * part of MHD_get_connection_values; very large POST * data *will* be made available incrementally in * upload_data) * @param upload_data_size set initially to the size of the * upload_data provided; the method must update this * value to the number of bytes NOT processed; * @param req_cls pointer that the callback can set to some * address and that will be preserved by MHD for future * calls for this request; since the access handler may * be called many times (i.e., for a PUT/POST operation * with plenty of upload data) this allows the application * to easily associate some request-specific state. * If necessary, this state can be cleaned up in the * global "MHD_RequestCompleted" callback (which * can be set with the MHD_OPTION_NOTIFY_COMPLETED). * Initially, <tt>*req_cls</tt> will be NULL. * @return MHS_YES if the connection was handled successfully, * MHS_NO if the socket must be closed due to a serious * error while handling the request */ static enum MHD_Result create_response (void *cls, struct MHD_Connection *connection, const char *url, const char *method, const char *version, const char *upload_data, size_t *upload_data_size, void **req_cls) { struct MHD_Response *response; struct Request *request; struct Session *session; enum MHD_Result ret; unsigned int i; (void) cls; /* Unused. Silent compiler warning. */ (void) version; /* Unused. Silent compiler warning. */ request = *req_cls; if (NULL == request) { request = calloc (1, sizeof (struct Request)); if (NULL == request) { fprintf (stderr, "calloc error: %s\n", strerror (errno)); return MHD_NO; } *req_cls = request; if (0 == strcmp (method, MHD_HTTP_METHOD_POST)) { request->pp = MHD_create_post_processor (connection, 1024, &post_iterator, request); if (NULL == request->pp) { fprintf (stderr, "Failed to setup post processor for `%s'\n", url); return MHD_NO; /* internal error */ } } return MHD_YES; } if (NULL == request->session) { request->session = get_session (connection); if (NULL == request->session) { fprintf (stderr, "Failed to setup session for `%s'\n", url); return MHD_NO; /* internal error */ } } session = request->session; session->start = time (NULL); if (0 == strcmp (method, MHD_HTTP_METHOD_POST)) { /* evaluate POST data */ MHD_post_process (request->pp, upload_data, *upload_data_size); if (0 != *upload_data_size) { *upload_data_size = 0; return MHD_YES; } /* done with POST data, serve response */ MHD_destroy_post_processor (request->pp); request->pp = NULL; method = MHD_HTTP_METHOD_GET; /* fake 'GET' */ if (NULL != request->post_url) url = request->post_url; } if ( (0 == strcmp (method, MHD_HTTP_METHOD_GET)) || (0 == strcmp (method, MHD_HTTP_METHOD_HEAD)) ) { /* find out which page to serve */ i = 0; while ( (pages[i].url != NULL) && (0 != strcmp (pages[i].url, url)) ) i++; ret = pages[i].handler (pages[i].handler_cls, pages[i].mime, session, connection); if (ret != MHD_YES) fprintf (stderr, "Failed to create page for `%s'\n", url); return ret; } /* unsupported HTTP method */ response = MHD_create_response_from_buffer_static (strlen (METHOD_ERROR), (const void *) METHOD_ERROR); ret = MHD_queue_response (connection, MHD_HTTP_NOT_ACCEPTABLE, response); MHD_destroy_response (response); return ret; } /** * Callback called upon completion of a request. * Decrements session reference counter. * * @param cls not used * @param connection connection that completed * @param req_cls session handle * @param toe status code */ static void request_completed_callback (void *cls, struct MHD_Connection *connection, void **req_cls, enum MHD_RequestTerminationCode toe) { struct Request *request = *req_cls; (void) cls; /* Unused. Silent compiler warning. */ (void) connection; /* Unused. Silent compiler warning. */ (void) toe; /* Unused. Silent compiler warning. */ printf("Connection completed callback called\n"); if (NULL == request) return; if (NULL != request->session) request->session->rc--; if (NULL != request->pp) MHD_destroy_post_processor (request->pp); free (request); } /** * Clean up handles of sessions that have been idle for * too long. */ static void expire_sessions (void) { struct Session *pos; struct Session *prev; struct Session *next; time_t now; now = time (NULL); prev = NULL; pos = sessions; while (NULL != pos) { next = pos->next; if (now - pos->start > 60 * 60) { /* expire sessions after 1h */ if (NULL == prev) sessions = pos->next; else prev->next = next; free (pos); } else prev = pos; pos = next; } } /** * Call with the port number as the only argument. * Never terminates (other than by signals, such as CTRL-C). */ int main (int argc, char *const *argv) { struct MHD_Daemon *d; struct timeval tv; struct timeval *tvp; fd_set rs; fd_set ws; fd_set es; MHD_socket max; uint64_t mhd_timeout; int port; if (argc != 2) { printf ("%s PORT\n", argv[0]); return 1; } port = atoi (argv[1]); if ( (1 > port) || (port > 65535) ) { fprintf (stderr, "Port must be a number between 1 and 65535.\n"); return 1; } /* initialize PRNG */ srand ((unsigned int) time (NULL)); d = MHD_start_daemon (MHD_USE_ERROR_LOG | MHD_USE_EPOLL_INTERNAL_THREAD, (uint16_t) port, NULL, NULL, &create_response, NULL, MHD_OPTION_CONNECTION_TIMEOUT, (unsigned int) 15, MHD_OPTION_NOTIFY_COMPLETED, &request_completed_callback, NULL, MHD_OPTION_END); if (NULL == d) return 1; while (1) { sleep(1); } if (0) { expire_sessions (); max = 0; FD_ZERO (&rs); FD_ZERO (&ws); FD_ZERO (&es); //if (MHD_YES != MHD_get_fdset (d, &rs, &ws, &es, &max)) //break; /* fatal internal error */ if (MHD_get_timeout64 (d, &mhd_timeout) == MHD_YES) { #if ! defined(_WIN32) || defined(__CYGWIN__) tv.tv_sec = (time_t) (mhd_timeout / 1000LL); #else /* Native W32 */ tv.tv_sec = (long) (mhd_timeout / 1000LL); #endif /* Native W32 */ tv.tv_usec = ((long) (mhd_timeout % 1000)) * 1000; tvp = &tv; } else tvp = NULL; if (-1 == select ((int) max + 1, &rs, &ws, &es, tvp)) { if (EINTR != errno) abort (); } MHD_run (d); } MHD_stop_daemon (d); return 0; } |
|
Fix committed to master branch. |
|
Ok, I think I have finally figured it out: one of the callbacks was indeed called, but only after the 15 timeout. I can get the same behavior as for POLL (instant timeout, both functions called) if I add EPOLLRDHUP to our epoll() flags when we are doing EPOLLIN (waiting to read). Which makes sense: we do not ONLY care to read data, but also to be informed if the other side (unexpectedly) closes the socket. Patch is in 4627d005..08f933a6. Please let me know if this does not fix it. |
Date Modified | Username | Field | Change |
---|---|---|---|
2022-03-11 11:40 | dro | New Issue | |
2022-03-15 02:06 | Christian Grothoff | Note Added: 0018791 | |
2022-03-17 12:07 | Christian Grothoff | Note Added: 0018802 | |
2022-07-13 13:31 | dro | Note Added: 0018955 | |
2022-07-13 13:31 | dro | File Added: post_example.c | |
2023-02-08 15:26 | Christian Grothoff | Changeset attached | => libmicrohttpd master 08f933a6 |
2023-02-08 15:26 | Christian Grothoff | Note Added: 0019780 | |
2023-02-08 15:26 | Christian Grothoff | Assigned To | => Christian Grothoff |
2023-02-08 15:26 | Christian Grothoff | Status | new => resolved |
2023-02-08 15:26 | Christian Grothoff | Resolution | open => fixed |
2023-02-08 15:26 | Christian Grothoff | Note Added: 0019781 | |
2023-02-08 15:26 | Christian Grothoff | Fixed in Version | => 0.9.76 |
2023-02-08 15:26 | Christian Grothoff | Target Version | => 0.9.76 |
2024-01-21 13:24 | Christian Grothoff | Category | libmicrohttpd internal select => internal event loop |
2024-01-21 13:33 | Christian Grothoff | Status | resolved => closed |