Re: cvs commit: apr/threadproc/win32 proc.c

[William A. Rowe, Jr.]

At 05:25 PM 7/2/2002,  wrote:
  Modified:    threadproc/win32 proc.c
  Log:
  Reverting the 1.76 and 1.77 changes, because they didn't work.
  The child handles weren't properly inheritable, and redirected command
  output got lost in the bit bucket.
On Which Flavor of Win32?  Tests on XP and 2K indicated this works.
  -        if (rv == APR_SUCCESS)
  -            apr_file_inherit_set(attr->child_in);
Of course apr_file_dup[2] handles aren't inheritable, only the 
child_[in|out|err]
flavors needed to be.

If there is a problem, it is NOT in this patch you reverted.  It is probably
localized to apr_file_inherit_set().  That API didn't exist when the original
'make inheritable duplicates' was added.
And you are now passing cloned parent-side handles again to the child
process which means the parent can't signal the file closed, because
closing the parent handle doesn't close the handle in the child process.
Please don't back out patches unless you don't get a response to a question
for a few hours, at least :-/  I've been reading the list about every two hours
all day... I didn't see any comments about this before.

  Revision  Changes    Path
  1.78      +81 -56    apr/threadproc/win32/proc.c


   APR_DECLARE(apr_status_t) apr_procattr_child_in_set(apr_procattr_t *attr,
                                                     apr_file_t *child_in,
                                                     apr_file_t *parent_in)
   {
  -    apr_status_t rv = APR_SUCCESS;
  -
  -    if (child_in) {
  -        if (attr->child_in == NULL)
  -            rv = apr_file_dup(&attr->child_in, child_in, attr->pool);
  -        else
  -            rv = apr_file_dup2(attr->child_in, child_in, attr->pool);
  +    apr_status_t stat;
  -        if (rv == APR_SUCCESS)
  -            apr_file_inherit_set(attr->child_in);
  +    if (attr->child_in == NULL && attr->parent_in == NULL) {
  +        stat = open_nt_process_pipe(&attr->child_in, &attr->parent_in,
  +                                    APR_FULL_BLOCK,
  +                                    attr->pool);
  +        if (stat == APR_SUCCESS)
  +            stat = make_handle_private(attr->parent_in);
  +        if (stat != APR_SUCCESS)
  +            return stat;
       }
  -    if (parent_in && rv == APR_SUCCESS) {
  -        if (attr->parent_in == NULL)
  -            rv = apr_file_dup(&attr->parent_in, parent_in, attr->pool);
  -        else
  -            rv = apr_file_dup2(attr->parent_in, parent_in, attr->pool);
  -    }
  +    stat = make_inheritable_duplicate (child_in, attr->child_in);
  +    if (stat == APR_SUCCESS)
  +        stat = make_inheritable_duplicate (parent_in, attr->parent_in);
  -    return rv;
  +    return stat;
   }