Anyone care to apply this to unix and hack it a bit? I only found four cases
where
we (probably) should be calling apr_file_open() to create an inheritable file.
As I mention, sockets are still an issue, so are child processes, shared memory
handles, and a list of others. But if we knock these off one bit at a time, we
will
probably knock down the child handle abuse.
Serious (?) bug I noticed, if a buffered file has pending data to write, and a
child
is spawned, both the parent _AND_ child will attempt to flush the buffer. It
used
to happen when the child pool was destroyed, with this patch it will occur when
the
process is spawned. It appears we need a pre-fork() registered action, to flush
buffers and the like. That's only partially effective though, unless we do some
heavy mutex protection :(
I'm still traveling till Sunday morning, so I can't revisit for a bit.
----- Original Message -----
From: "William A. Rowe, Jr." <[EMAIL PROTECTED]>
To: <dev@apr.apache.org>
Sent: Friday, July 13, 2001 3:45 PM
Subject: Inherited Handles and APR
> After a respectable lunch at Boudin's, Ryan and I think we have the general
> answers
> to child handles.
>
> The apr_foo_open/create calls need an APR_INHERIT flag bit to mark resources
> as
> inheritable. This offers two advantages;
>
> 1. the app doesn't need to worry about fork/exec cleanups v.s. inherit bits
> for
> createprocess() based Win32 (and other non-forks.)
>
> 2. mainline code is more readable, as the user no longer registers their own
> cleanup_for_exec handlers to simply close handles.
>
> The patch, for apr_file_open, follows. Rbb is working on a similar for
> Sockets, and
> calls that generate 'handles' will need to be reviewed as well...
>
> I obviously have the much larger job of applying the APR_INHERIT flag
> judiciously
> throughout the entire server and removing child cleanups where appropriate ...
>
> Bill
>
Index: file_io/unix/open.c
===================================================================
RCS file: /home/cvs/apr/file_io/unix/open.c,v
retrieving revision 1.77
diff -u -r1.77 open.c
--- file_io/unix/open.c 2001/06/27 19:40:34 1.77
+++ file_io/unix/open.c 2001/07/13 20:41:08
@@ -167,7 +167,8 @@
(*new)->dataRead = 0;
(*new)->direction = 0;
apr_pool_cleanup_register((*new)->cntxt, (void *)(*new),
apr_unix_file_cleanup,
- apr_pool_cleanup_null);
+ (flag & APR_INHERIT) ? apr_pool_cleanup_null
+ : apr_unix_file_cleanup);
return APR_SUCCESS;
}
Index: file_io/win32/open.c
===================================================================
RCS file: /home/cvs/apr/file_io/win32/open.c,v
retrieving revision 1.77
diff -u -r1.77 open.c
--- file_io/win32/open.c 2001/06/27 19:40:41 1.77
+++ file_io/win32/open.c 2001/07/13 20:41:10
@@ -173,6 +173,7 @@
apr_int32_t flag, apr_fileperms_t perm,
apr_pool_t *cont)
{
+ SECURITY_ATTRIBUTES sa, *psa = NULL;
/* XXX: The default FILE_FLAG_SEQUENTIAL_SCAN is _wrong_ for
* sdbm and any other random files! We _must_ rethink
* this approach.
@@ -236,6 +237,12 @@
*/
attributes |= FILE_FLAG_OVERLAPPED;
}
+ if (flag & APR_INHERIT) {
+ sa.nLength = sizeof(sa);
+ sa.bInheritHandle = TRUE;
+ sa.lpSecurityDescriptor = NULL;
+ psa = &sa;
+ }
#if APR_HAS_UNICODE_FS
if (os_level >= APR_WIN_NT) {
@@ -244,12 +251,12 @@
/ sizeof(apr_wchar_t), fname))
return rv;
handle = CreateFileW(wfname, oflags, sharemode,
- NULL, createflags, attributes, 0);
+ psa, createflags, attributes, 0);
}
else
#endif
handle = CreateFileA(fname, oflags, sharemode,
- NULL, createflags, attributes, 0);
+ psa, createflags, attributes, 0);
if (handle == INVALID_HANDLE_VALUE) {
return apr_get_os_error();
Index: include/apr_file_io.h
===================================================================
RCS file: /home/cvs/apr/include/apr_file_io.h,v
retrieving revision 1.103
diff -u -r1.103 apr_file_io.h
--- include/apr_file_io.h 2001/06/27 19:40:46 1.103
+++ include/apr_file_io.h 2001/07/13 20:41:13
@@ -89,6 +89,10 @@
#define APR_SHARELOCK 1024 /* Platform dependent support for higher
level locked read/write access to support
writes across process/machines */
+#define APR_INHERIT 4096 /* Create the file inheritable by the child
+ process. fork()ed implementations
+ automatically register a child cleanup
+ in the _absence_ of this flag. */
/* flags for apr_file_seek */
#define APR_SET SEEK_SET
@@ -141,6 +145,10 @@
* APR_SHARELOCK Platform dependent support for higher
* level locked read/write access to support
* writes across process/machines
+ * APR_INHERIT Create the file inheritable by the child
+ * processes. fork()ed implementations
+ * automatically register a child cleanup
+ * in the _absence_ of this flag.
* </PRE>
* @param perm Access permissions for file.
* @param cont The pool to use.
Index: modules/cache/mod_file_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/cache/mod_file_cache.c,v
retrieving revision 1.56
diff -u -r1.56 mod_file_cache.c
--- modules/cache/mod_file_cache.c 2001/06/27 20:18:03 1.56
+++ modules/cache/mod_file_cache.c 2001/07/14 06:57:56
@@ -207,7 +207,7 @@
return;
}
- rc = apr_file_open(&fd, fspec, APR_READ | APR_BINARY | APR_XTHREAD,
+ rc = apr_file_open(&fd, fspec, APR_READ | APR_BINARY | APR_XTHREAD |
APR_INHERIT,
APR_OS_DEFAULT, cmd->pool);
if (rc != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_WARNING, rc, cmd->server,
Index: modules/loggers/mod_log_config.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/loggers/mod_log_config.c,v
retrieving revision 1.60
diff -u -r1.60 mod_log_config.c
--- modules/loggers/mod_log_config.c 2001/05/22 01:31:06 1.60
+++ modules/loggers/mod_log_config.c 2001/07/14 06:58:18
@@ -206,7 +206,7 @@
module AP_MODULE_DECLARE_DATA log_config_module;
-static int xfer_flags = (APR_WRITE | APR_APPEND | APR_CREATE);
+static int xfer_flags = (APR_WRITE | APR_APPEND | APR_CREATE | APR_INHERIT);
static apr_fileperms_t xfer_perms = APR_OS_DEFAULT;
static apr_hash_t *log_hash;
Index: modules/mappers/mod_rewrite.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_rewrite.c,v
retrieving revision 1.77
diff -u -r1.77 mod_rewrite.c
--- modules/mappers/mod_rewrite.c 2001/05/18 18:38:41 1.77
+++ modules/mappers/mod_rewrite.c 2001/07/14 06:58:31
@@ -3096,7 +3096,7 @@
const char *fname;
apr_status_t rc;
piped_log *pl;
- int rewritelog_flags = ( APR_WRITE | APR_APPEND | APR_CREATE );
+ int rewritelog_flags = ( APR_WRITE | APR_APPEND | APR_CREATE |
APR_INHERIT );
apr_fileperms_t rewritelog_mode = ( APR_UREAD | APR_UWRITE | APR_GREAD |
APR_WREAD );
conf = ap_get_module_config(s->module_config, &rewrite_module);
Index: server/log.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/log.c,v
retrieving revision 1.93
diff -u -r1.93 log.c
--- server/log.c 2001/06/06 22:24:54 1.93
+++ server/log.c 2001/07/14 06:58:47
@@ -266,7 +266,7 @@
else {
fname = ap_server_root_relative(p, s->error_fname);
rc = apr_file_open(&s->error_log, fname,
- APR_APPEND | APR_READ | APR_WRITE | APR_CREATE,
+ APR_APPEND | APR_READ | APR_WRITE | APR_CREATE |
APR_INHERIT,
APR_OS_DEFAULT, p);
if (rc != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL,
