Some of the potential failures could be discovered in the parent by using extra syscalls (e.g., use stat to make sure the program actually exists and can be executed by this user, use stat to make sure the desired working directory actually exists and can be chdir-ed to by this user). It isn't appropriate to burn the required cycles by default, but it would be nice to allow the app to turn on this extra checking, since failures of apr_proc_create() can be handled much cleaner if the function returns an error status.
Alternatively, APR could allow the application to get called in the child process in the failure cases and allow it to do whatever is appropriate (log a message, synchronize with the parent process, etc.).
I think both features are important. Using Apache as an example, I would imagine that for situations where apr_proc_create() is not called frequently (e.g., piped log program), it would be useful to turn on any available extra checking in the parent so that the best possible diagnostics can be given to the admin with minimal extra coding in Apache. Such extra overhead is of no concern for a path that runs so infrequently. On the other hand, when running CGIs (something the server may do very frequently), any extra overhead would be inappropriate given that we expect it to work in general.
This particular patch allows the latter approach -- let the app get involved when apr_proc_create() fails in the child. This is useful with or without the ability to try to find potential errors in the parent since not everything can be checked.
Any concerns, particularly with respect to how the app determines if the feature is available? I think it would be fine to make the feature always available but to simply note that on some platforms the application-specified error function would never be called.
Index: include/apr.h.in
===================================================================
RCS file: /home/cvs/apr/include/apr.h.in,v
retrieving revision 1.118
diff -u -r1.118 apr.h.in
--- include/apr.h.in 22 Jan 2003 18:25:59 -0000 1.118
+++ include/apr.h.in 5 Feb 2003 14:40:52 -0000
@@ -139,6 +139,7 @@
#define APR_HAS_SENDFILE @sendfile@
#define APR_HAS_MMAP @mmap@
#define APR_HAS_FORK @fork@
+#define APR_HAS_CHILD_ERRFN @fork@
#define APR_HAS_RANDOM @rand@
#define APR_HAS_OTHER_CHILD @oc@
#define APR_HAS_DSO @aprdso@
Index: include/apr.hnw
===================================================================
RCS file: /home/cvs/apr/include/apr.hnw,v
retrieving revision 1.28
diff -u -r1.28 apr.hnw
--- include/apr.hnw 27 Jan 2003 17:09:09 -0000 1.28
+++ include/apr.hnw 5 Feb 2003 14:40:52 -0000
@@ -207,6 +207,7 @@
#define APR_HAS_SENDFILE 0
#define APR_HAS_MMAP 0
#define APR_HAS_FORK 0
+#define APR_HAS_CHILD_ERRFN 0
#define APR_HAS_RANDOM 1
#define APR_HAS_OTHER_CHILD 0
#define APR_HAS_DSO 1
Index: include/apr.hw
===================================================================
RCS file: /home/cvs/apr/include/apr.hw,v
retrieving revision 1.107
diff -u -r1.107 apr.hw
--- include/apr.hw 28 Jan 2003 21:11:22 -0000 1.107
+++ include/apr.hw 5 Feb 2003 14:40:52 -0000
@@ -280,6 +280,7 @@
#define APR_HAS_THREADS 1
#define APR_HAS_MMAP 1
#define APR_HAS_FORK 0
+#define APR_HAS_CHILD_ERRFN 0
#define APR_HAS_RANDOM 1
#define APR_HAS_OTHER_CHILD 0
#define APR_HAS_DSO 1
Index: include/apr_thread_proc.h
===================================================================
RCS file: /home/cvs/apr/include/apr_thread_proc.h,v
retrieving revision 1.90
diff -u -r1.90 apr_thread_proc.h
--- include/apr_thread_proc.h 4 Feb 2003 20:10:34 -0000 1.90
+++ include/apr_thread_proc.h 5 Feb 2003 14:40:52 -0000
@@ -177,6 +177,20 @@
#endif
};
+#if APR_HAS_CHILD_ERRFN
+/**
+ * The prototype for APR child errfn functions. It is passed the
+ * following information:
+ * @param proc Representation of proc that couldn't be created
+ * @param err APR error code describing the error
+ * @param description Text description of type of processing which failed
+ * @param userdata Value specified by application on call to
+ * apr_procattr_child_errfn_set()
+ */
+typedef void (apr_child_errfn_t)(apr_proc_t *proc, apr_status_t err,
+ const char *description, void *userdata);
+#endif
+
/** Opaque Thread structure. */
typedef struct apr_thread_t apr_thread_t;
/** Opaque Thread attributes structure. */
@@ -485,6 +499,21 @@
APR_DECLARE(apr_status_t) apr_procattr_limit_set(apr_procattr_t *attr,
apr_int32_t what,
struct rlimit *limit);
+#endif
+
+#if APR_HAS_CHILD_ERRFN
+/**
+ * Specify an error function to be called in the child process if APR
+ * encounters an error in the child prior to running the specified program.
+ * @param child_errfn The function to call in the child process.
+ * @param userdata Parameter to be passed to errfn.
+ * @remark At the present time, it is only useful and only implemented
+ * on platforms that have fork(). Programs should reference the feature
+ * test macro APR_HAS_CHILD_ERRFN.
+ */
+APR_DECLARE(apr_status_t) apr_procattr_child_errfn_set(apr_procattr_t *attr,
+ apr_child_errfn_t
*errfn,
+ void *userdata);
#endif
#if APR_HAS_FORK
Index: include/arch/unix/apr_arch_threadproc.h
===================================================================
RCS file: /home/cvs/apr/include/arch/unix/apr_arch_threadproc.h,v
retrieving revision 1.3
diff -u -r1.3 apr_arch_threadproc.h
--- include/arch/unix/apr_arch_threadproc.h 7 Jan 2003 00:52:54 -0000
1.3
+++ include/arch/unix/apr_arch_threadproc.h 5 Feb 2003 14:40:52 -0000
@@ -134,6 +134,8 @@
#ifdef RLIMIT_NOFILE
struct rlimit *limit_nofile;
#endif
+ apr_child_errfn_t *errfn;
+ void *errfn_userdata;
};
#endif /* ! THREAD_PROC_H */
Index: threadproc/unix/proc.c
===================================================================
RCS file: /home/cvs/apr/threadproc/unix/proc.c,v
retrieving revision 1.63
diff -u -r1.63 proc.c
--- threadproc/unix/proc.c 4 Feb 2003 20:12:29 -0000 1.63
+++ threadproc/unix/proc.c 5 Feb 2003 14:40:52 -0000
@@ -295,6 +295,15 @@
return APR_SUCCESS;
}
+APR_DECLARE(apr_status_t) apr_procattr_child_errfn_set(apr_procattr_t *attr,
+ apr_child_errfn_t
*errfn,
+ void *userdata)
+{
+ attr->errfn = errfn;
+ attr->errfn_userdata = userdata;
+ return APR_SUCCESS;
+}
+
APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
const char *progname,
const char * const *args,
@@ -368,11 +377,19 @@
if (attr->currdir != NULL) {
if (chdir(attr->currdir) == -1) {
+ if (attr->errfn) {
+ attr->errfn(new, errno, "change of working directory
failed",
+ attr->errfn_userdata);
+ }
exit(-1); /* We have big problems, the child should exit. */
}
}
if ((status = limit_proc(attr)) != APR_SUCCESS) {
+ if (attr->errfn) {
+ attr->errfn(new, errno, "setting of resource limits failed",
+ attr->errfn_userdata);
+ }
exit(-1); /* We have big problems, the child should exit. */
}
@@ -422,6 +439,14 @@
execvp(progname, (char * const *)args);
}
+ if (attr->errfn) {
+ char *desc;
+
+ desc = apr_psprintf(pool, "exec of '%s' failed",
+ progname);
+ attr->errfn(new, errno, desc, attr->errfn_userdata);
+ }
+
exit(-1); /* if we get here, there is a problem, so exit with an
* error code. */
}
