pqf wrote:
version 1.10 ( Jul 3rd 2006 ) 1. Use poll() instead of select() in UNIX. It becomes problematic on apache2 with large number of logfiles. Apache2 calls poll() (when OS supports it), and in that case it doesn't need to be recompiled with larger FD_SETSIZE. select() is still limited to FD_SETSIZE." (Thank Piotr Gackiewicz gacek at intertele.pl for the patch.) 2. Bug fix: "Some requests fail with HTTP 500 and no errorlog entry is generated" (Thank Piotr Gackiewicz gacek at intertele.pl for the patch.)
Ryan has been in touch with Piotr Gackiewicz independently and Piotr asks if we can confirm that a CLA and SGA are necessary, as he considers his contribution to have been just "simple repairs" (his term). From looking over the CVS repository at http://mod-fcgid.cvs.sourceforge.net/viewvc/mod-fcgid/mod_fcgid/ it would appear to me that these patches amount to the following. Foes anyone have a sense of whether these would indeed require a CLA and SGA? Chris. ================================================= --- fcgid_bridge.c 2006/01/22 14:16:23 1.25 +++ fcgid_bridge.c 2006/05/13 23:45:44 1.26 @@ -256,8 +256,11 @@ } bucket_ctx = apr_pcalloc(request_pool, sizeof(*bucket_ctx)); - if (!bucket_ctx) + if (!bucket_ctx) { + ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server, + "mod_fcgid: apr_calloc of %d bytes failed in handle_request function", sizeof(*bucket_ctx)); return HTTP_INTERNAL_SERVER_ERROR; + } bucket_ctx->ipc.connect_timeout = g_connect_timeout; bucket_ctx->ipc.communation_timeout = g_comm_timeout; bucket_ctx->ipc.request = r; @@ -315,7 +318,7 @@ /* Now I get a connected ipc handle */ if (!bucket_ctx->procnode) { - ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server, + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, r->server, "mod_fcgid: can't apply process slot for %s", argv0); return HTTP_SERVICE_UNAVAILABLE; } @@ -326,7 +329,7 @@ if ((rv = proc_write_ipc(main_server, &bucket_ctx->ipc, output_brigade)) != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_INFO, rv, r->server, + ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server, "mod_fcgid: write data to fastcgi server error"); bucket_ctx->has_error = 1; return HTTP_INTERNAL_SERVER_ERROR; @@ -335,8 +338,11 @@ /* Create brigade */ brigade_stdout = apr_brigade_create(request_pool, r->connection->bucket_alloc); - if (!brigade_stdout) + if (!brigade_stdout) { + ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server, + "mod_fcgid: apr_brigade_create failed in handle_request function"); return HTTP_INTERNAL_SERVER_ERROR; + } APR_BRIGADE_INSERT_TAIL(brigade_stdout, ap_bucket_fcgid_header_create(r->connection-> bucket_alloc, @@ -346,7 +352,11 @@ /* Check the script header first. If got error, return immediately */ if ((cond_status = ap_scan_script_header_err_core (r, sbuf, getsfunc_fcgid_BRIGADE, brigade_stdout)) >= 400) + { + ap_log_error(APLOG_MARK, APLOG_INFO, rv, r->server, + "mod_fcgid: ap_scan_script_header_err_core failed in handle_request function: %d", cond_status); return cond_status; + } /* Check redirect */ location = apr_table_get(r->headers_out, "Location"); @@ -377,6 +387,9 @@ if ((rv = ap_pass_brigade(r->output_filters, brigade_stdout)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server, + "mod_fcgid: ap_pass_brigade failed in handle_request function"); + return HTTP_INTERNAL_SERVER_ERROR; } @@ -437,7 +450,7 @@ AP_MODE_READBYTES, APR_BLOCK_READ, HUGE_STRING_LEN)) != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_INFO, rv, + ap_log_error(APLOG_MARK, APLOG_WARNING, rv, main_server, "mod_fcgid: can't get data from http client"); apr_brigade_destroy(output_brigade); --- arch/unix/fcgid_proc_unix.c 2006/01/22 14:16:23 1.27 +++ arch/unix/fcgid_proc_unix.c 2006/05/13 23:45:44 1.28 @@ -2,6 +2,7 @@ #include <sys/un.h> #include <sys/types.h> #include <netinet/tcp.h> /* For TCP_NODELAY */ +#include <sys/poll.h> #define CORE_PRIVATE #include "httpd.h" #include "apr_thread_proc.h" @@ -525,10 +526,9 @@ fcgid_ipc * ipc_handle, const char *buffer, apr_size_t * size) { - fd_set rset; - struct timeval tv; int retcode, unix_socket; fcgid_namedpipe_handle *handle_info; + struct pollfd pollfds[1]; handle_info = (fcgid_namedpipe_handle *) ipc_handle->ipc_handle_info; unix_socket = handle_info->handle_socket; @@ -547,17 +547,16 @@ } /* I have to wait a while */ - FD_ZERO(&rset); - FD_SET(unix_socket, &rset); + + pollfds[0].fd = unix_socket; + pollfds[0].events = POLLIN; do { - tv.tv_usec = 0; - tv.tv_sec = ipc_handle->communation_timeout; - retcode = select(unix_socket + 1, &rset, NULL, NULL, &tv); - } while (retcode == -1 && APR_STATUS_IS_EINTR(errno)); + retcode = poll(pollfds, 1, ipc_handle->communation_timeout*1000); + } while (retcode <= 0 && APR_STATUS_IS_EINTR(errno)); if (retcode == -1) { - ap_log_error(APLOG_MARK, APLOG_INFO, errno, + ap_log_error(APLOG_MARK, APLOG_ERR, errno, main_server, - "mod_fcgid: select unix domain socket error"); + "mod_fcgid: poll unix domain socket error"); return errno; } else if (retcode == 0) { ap_log_error(APLOG_MARK, APLOG_INFO, 0, @@ -567,7 +566,6 @@ return APR_ETIMEDOUT; } - /* Read again after select() */ do { if ((retcode = read(unix_socket, (void *) buffer, *size)) > 0) { *size = retcode; @@ -592,10 +590,9 @@ struct iovec *vec, int nvec, int *writecnt) { - fd_set wset; - struct timeval tv; int retcode, unix_socket; fcgid_namedpipe_handle *handle_info; + struct pollfd pollfds[1]; handle_info = (fcgid_namedpipe_handle *) ipc_handle->ipc_handle_info; unix_socket = handle_info->handle_socket; @@ -610,14 +607,12 @@ if (!APR_STATUS_IS_EAGAIN(errno)) return errno; - /* Select() */ - FD_ZERO(&wset); - FD_SET(unix_socket, &wset); + /* poll() */ + pollfds[0].fd = unix_socket; + pollfds[0].events = POLLOUT; do { - tv.tv_usec = 0; - tv.tv_sec = ipc_handle->communation_timeout; - retcode = select(unix_socket + 1, NULL, &wset, NULL, &tv); - } while (retcode == -1 && APR_STATUS_IS_EINTR(errno)); + retcode = poll(pollfds, 1, ipc_handle->communation_timeout*1000); + } while (retcode <= 0 && APR_STATUS_IS_EINTR(errno)); if (retcode == -1) return errno; =================================================
