Re: cvs commit: httpd-2.0/modules/generators mod_cgid.c
[EMAIL PROTECTED] wrote: +#ifdef _AIX see related thread [PATCH] apr function to see if an arbitrary process is alive on [EMAIL PROTECTED]
Re: cvs commit: httpd-2.0/modules/generators mod_cgid.c
+1 for 2.0.45
Brian
On Sun, 2003-03-30 at 20:44, [EMAIL PROTECTED] wrote:
wrowe 2003/03/30 20:44:11
Modified:modules/generators mod_cgid.c
Log:
Solve segfaults from unusual error exceptions in cgid. The daemon
has no 'real' request_rec, so we can't use ap_log_rerror() anywhere
within the cgid_server() code.
Also, one of the two log messages was echoed to the child, no point
when the actual request logic should take care of that notification.
Submitted by: Jeff Trawick
Reviewed by:Bill Rowe
Revision ChangesPath
1.149 +12 -6 httpd-2.0/modules/generators/mod_cgid.c
Index: mod_cgid.c
===
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgid.c,v
retrieving revision 1.148
retrieving revision 1.149
diff -u -r1.148 -r1.149
--- mod_cgid.c 12 Feb 2003 18:27:37 - 1.148
+++ mod_cgid.c 31 Mar 2003 04:44:11 - 1.149
@@ -735,8 +735,11 @@
((rc = apr_procattr_cmdtype_set(procattr, cmd_type)) != APR_SUCCESS)
||
((rc = apr_procattr_child_errfn_set(procattr, cgid_child_errfn)) !=
APR_SUCCESS)) {
/* Something bad happened, tell the world. */
-ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r,
- couldn't set child process attributes: %s, r-filename);
+ * ap_log_rerror() won't work because the header table used by
+ * ap_log_rerror() hasn't been replicated in the phony r
+ */
+ap_log_error(APLOG_MARK, APLOG_ERR, rc, r-server,
+ couldn't set child process attributes: %s,
r-filename);
}
else {
apr_pool_userdata_set(r, ERRFN_USERDATA_KEY, apr_pool_cleanup_null,
ptrans);
@@ -757,10 +760,13 @@
procattr, ptrans);
if (rc != APR_SUCCESS) {
-/* Bad things happened. Everyone should have cleaned up. */
-ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_TOCLIENT, rc, r,
- couldn't create child process: %d: %s, rc,
- apr_filename_of_pathname(r-filename));
+/* Bad things happened. Everyone should have cleaned up.
+ * ap_log_rerror() won't work because the header table used by
+ * ap_log_rerror() hasn't been replicated in the phony r
+ */
+ap_log_error(APLOG_MARK, APLOG_ERR, rc, r-server,
+ couldn't create child process: %d: %s, rc,
+ apr_filename_of_pathname(r-filename));
}
else {
apr_hash_set(script_hash, cgid_req.conn_id,
sizeof(cgid_req.conn_id),
Re: cvs commit: httpd-2.0/modules/generators mod_cgid.c
jerenkrantz02/05/29 22:42:46
Modified:modules/generators mod_cgid.c
Log:
Apply same patch to mod_cgid that was applied to mod_cgi so that it
bypasses the *_client_block calls in favor of using the input filters
and brigades directly.
Revision ChangesPath
1.134 +70 -35httpd-2.0/modules/generators/mod_cgid.c
Index: mod_cgid.c
===
snip
+do {
+apr_bucket *bucket;
+apr_status_t rv;
+
+rv = ap_get_brigade(r-input_filters, bb, AP_MODE_READBYTES,
+APR_BLOCK_READ, HUGE_STRING_LEN);
+
+if (rv != APR_SUCCESS) {
+return rv;
+}
+
+APR_BRIGADE_FOREACH(bucket, bb) {
+const char *data;
+apr_size_t len;
+
+if (APR_BUCKET_IS_EOS(bucket)) {
+seen_eos = 1;
+break;
+}
+
+/* We can't do much with this. */
+if (APR_BUCKET_IS_FLUSH(bucket)) {
+continue;
+}
+
+/* If the child stopped, we still must read to EOS. */
+if (child_stopped_reading) {
+continue;
+}
The above section of code looks like a potential endless loop if the brigade does not
contain an EOS bucket. Should the check for child_stopped_reading occur after the
apr_bucket_read below?
Bill
-if (ap_should_client_block(r)) {
-int dbsize, len_read;
+/* read */
+apr_bucket_read(bucket, data, len, APR_BLOCK_READ);
+
+if (conf-logname dbpos conf-bufbytes) {
+int cursize;
+
+if ((dbpos + len) conf-bufbytes) {
+cursize = conf-bufbytes - dbpos;
+}
+else {
+cursize = len;
+}
+memcpy(dbuf + dbpos, data, cursize);
+dbpos += cursize;
+}
+
+/* Keep writing data to the child until done or too much time
+ * elapses with no progress or an error occurs.
+ */
+rv = apr_file_write_full(tempsock, data, len, NULL);
+
+if (rv != APR_SUCCESS) {
+/* silly script stopped reading, soak up remaining message */
+child_stopped_reading = 1;
+}
+}
+apr_brigade_cleanup(bb);
+}
+while (!seen_eos);
+
+if (conf-logname) {
+dbuf[dbpos] = '\0';
+}
-if (conf-logname) {
-dbuf = apr_pcalloc(r-pool, conf-bufbytes + 1);
-dbpos = 0;
-}
-
-while ((len_read =
-ap_get_client_block(r, argsbuffer, HUGE_STRING_LEN)) 0) {
-if (conf-logname) {
-if ((dbpos + len_read) conf-bufbytes) {
-dbsize = conf-bufbytes - dbpos;
-}
-else {
-dbsize = len_read;
-}
-memcpy(dbuf + dbpos, argsbuffer, dbsize);
-dbpos += dbsize;
-}
-nbytes = len_read;
-apr_file_write(tempsock, argsbuffer, nbytes);
-if (nbytes len_read) {
-/* silly script stopped reading, soak up remaining message */
-while (ap_get_client_block(r, argsbuffer, HUGE_STRING_LEN) 0) {
-/* dump it */
-}
-break;
-}
-}
-}
/* we're done writing, or maybe we didn't write at all;
* force EOF on child's stdin so that the cgi detects end (or
* absence) of data
Re: cvs commit: httpd-2.0/modules/generators mod_cgid.c
On Thu, May 30, 2002 at 12:54:39PM -0400, Bill Stoddard wrote: The above section of code looks like a potential endless loop if the brigade does not contain an EOS bucket. Should the check for child_stopped_reading occur after the apr_bucket_read below? An EOS should be returned at some point - we get a brigade from the input filters, read through the brigade, if we didn't see EOS, then we repeat. So, I'm not concerned about that - we'll have to get EOS. If the client socket is disconnected or errors out, we'll return. However, on the second point, you may well be right. I wasn't 100% sure if the check should be done before or after the apr_bucket_read. Any reason why it should be after? I was trying to save the read call. -- justin
