On Wed, Nov 6, 2013 at 2:13 PM, Jeff Trawick <trawick@gmail.com> wrote:
On Wed, Nov 6, 2013 at 2:06 PM, Mike Rumph <mike.rumph@oracle.com> wrote:
The apr_file_dup2() function in apr/file_io/win32/filedup.c calls _commit() for standard file handles 0, 1 and 2.
The _commit() function will assert with the message "Invalid file descriptor. File possibly closed by a different thread" or return a value of -1 if the file handle refers to a device.
The assert will appear if APR is compiled in debug mode.

This can be seen by running "testall.exe testdup".
But if file redirection is used (such as "testall.exe testdup > outfile 2>&1"), then the test completes successfully.

I have attached a patch that corrects this problem by checking _isatty().

Credit goes to Jeff Trawick for catching this problem in the first place.

Well, there's a lot of unhappiness when you have to answer the abort/retry/ignore dialog when you run httpd with a debug flavor of apr ;)


Mike Rumph

The existing flush-stdin path sure looks funny now that it is highlighted by the format of the patch, but that's not related to this bug and google seems to think that it is acceptable on Windows.

I hope to commit shortly.  Thanks!

Born in Roswell... married an alien...

I just played with _commit() on stdin a bit.  It turns out that _commit(0) fails if stdin is redirected (main.exe < somefile) but works if stdin is a tty.  That's the opposite of _commit(1 or 2).  But I don't see how _commit(0) makes sense anyway, so I simply removed the call instead of reversing the corresponding _isatty() check in your patch.

trunk: r1539455
1.5.x branch: r1539461

Born in Roswell... married an alien...