At 10:48 AM 3/21/2003, Jeff Trawick wrote:
>Joe Orton wrote:
>>
>>In the r1.57 filedup.c change, a cleanup is registered for the "new"
>>fd coming out of apr_file_dup2(), which wasn't happening previously.
>>I'm guessing that this cleanup is closing fd 2 when pconf is cleared, or
>>something like that.
>
>backtracking uses of fd 2 up through this time is somewhat funny
>
>it looks like
>
>a) we set the error log to file descriptor 2
>
>b) we close file descriptor 2 and set it to /dev/null via the freopen() in
>apr_proc_detach
>
>c) we close file descriptor 2 (because of cleanup registered around step a??)
Bang. We are dup2()ing that fd, and that fd didn't have a cleanup
(in fact it was opened APR_FILE_NOCLOSE).
Attached is a patch that uses the logic
apr_file_dup() always registers a cleanup, inherited for 0..2,
otherwise not inherited by default.
apr_file_dup2() registers the same cleanup the original open()
or the toggled apr_file_inherit_[un]set had indicated. We still
don't trust the original cleanup, but register the 'proper' one
based on the desired behavior of that target apr_file_t.
Give it a whirl and I will commit if you folks concur.
Bill
Index: file_io/unix/filedup.c
===================================================================
RCS file: /home/cvs/apr/file_io/unix/filedup.c,v
retrieving revision 1.60
diff -u -r1.60 filedup.c
--- file_io/unix/filedup.c 19 Mar 2003 10:17:26 -0000 1.60
+++ file_io/unix/filedup.c 21 Mar 2003 17:31:58 -0000
@@ -117,16 +117,28 @@
/* make sure unget behavior is consistent */
(*new_file)->ungetchar = old_file->ungetchar;
- /* apr_file_dup() clears the inherit attribute for normal files,
- * but sets the inherit attribute for std[out,in,err] fd's.
- * The user must call apr_file_inherit_[un]set() on the dupped
- * apr_file_t when desired.
- */
- if ((*new_file)->filedes <= 2) {
- (*new_file)->flags = old_file->flags | APR_INHERIT;
+ if (which_dup == 1) {
+ /* apr_file_dup() clears the inherit attribute for normal files,
+ * but sets the inherit attribute for std[out,in,err] fd's.
+ * The user must call apr_file_inherit_[un]set() on the dupped
+ * apr_file_t when desired.
+ */
+ if ((*new_file)->filedes <= 2) {
+ (*new_file)->flags = old_file->flags | APR_INHERIT;
+ }
+ else {
+ (*new_file)->flags = old_file->flags & ~APR_INHERIT;
+ }
}
- else {
- (*new_file)->flags = old_file->flags & ~APR_INHERIT;
+ else /* which_dup == 2 */ {
+ /* apr_file_dup2() must respect the original settings for the
+ * new_file->flags inheritence. If there is no cleanup, we
+ * should be finished already, otherwise fall through to the
+ * apr_pool_cleanup_register
+ */
+ if ((*new_file)->flags & APR_FILE_NOCLEANUP) {
+ return APR_SUCCESS;
+ }
}
apr_pool_cleanup_register((*new_file)->pool, (void *)(*new_file),
