Eric Blake <ebb9 <at> byu.net> writes: > Well, it would be nice if doing that didn't hang mingw. It looks like > Microsoft has a bug, because using dup2(fd,fd) when fd is closed appears to > deadlock the process.
mingw also hangs on the second dup2 in dup2(fd,fd),dup2(fd,fd+1) if fd is open (meaning the first dup2, although it returned 0, manages to corrupt the state of fd). In short, using equal fd is a recipe for disaster on mingw (which is probably why Linux chose for dup3 to return EINVAL in that case). In thinking about things, should we try and treat the w32 concept of O_NOINHERIT like the Unix concept of FD_CLOEXEC? If so, then it might be worth implementing Linux's dup3, and making w32spawn.h's dup_noinherit be synonymous with a call to dup3(fd, target, FD_CLOEXEC). And to some degree, we almost have enough pieces in place to provide a semi-decent rpl_fcntl for mingw. Of course, without O_CLOEXEC support, any rpl_dup3 replacement would be racy, but it at least provides convenience for code that is thinking about cloexec issues for when the OS catches up. And at least dup3, like dup2, does not need a *_safer variant. At any rate, here's the patch that gets mingw through the testsuite without hanging: From: Eric Blake <e...@byu.net> Date: Tue, 21 Jul 2009 09:00:57 -0600 Subject: [PATCH] dup2: fix more mingw problems * lib/dup2.c (rpl_dup2) [_WIN32]: Avoid hanging when duplicating fd to itself. * doc/posix-functions/dup2.texi (dup2): Document the bug. * lib/unistd.in.h (dup2) [REPLACE_FCHDIR]: Avoid name collision. * lib/fchdir.c (dup2): Manage preprocessor macros correctly. (rpl_dup2_fchdir): Rename from rpl_dup2, and let dup2 module take care of mingw bugs. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 9 +++++++++ doc/posix-functions/dup2.texi | 4 ++++ lib/dup2.c | 31 +++++++++++++++++++++++++++---- lib/fchdir.c | 17 +++++++++-------- lib/unistd.in.h | 8 +++++--- 5 files changed, 54 insertions(+), 15 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6a89b40..2ede9ad 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2009-07-21 Eric Blake <e...@byu.net> + dup2: fix more mingw problems + * lib/dup2.c (rpl_dup2) [_WIN32]: Avoid hanging when duplicating + fd to itself. + * doc/posix-functions/dup2.texi (dup2): Document the bug. + * lib/unistd.in.h (dup2) [REPLACE_FCHDIR]: Avoid name collision. + * lib/fchdir.c (dup2): Manage preprocessor macros correctly. + (rpl_dup2_fchdir): Rename from rpl_dup2, and let dup2 module take + care of mingw bugs. + dup2-tests: test previous patch * modules/dup2-tests: New file. * tests/test-dup2.c: Likewise. diff --git a/doc/posix-functions/dup2.texi b/doc/posix-functions/dup2.texi index bfaff38..c8f2ca1 100644 --- a/doc/posix-functions/dup2.texi +++ b/doc/posix-functions/dup2.texi @@ -13,6 +13,10 @@ This function always returns 0 for success on some platforms: mingw. @item +This function can hang when duplicating an fd to itself on some platforms: +mingw. + +...@item This function returns 0 for dup2 (1, 1) on some platforms: Cygwin 1.5.x. diff --git a/lib/dup2.c b/lib/dup2.c index e5d3de4..6d61829 100644 --- a/lib/dup2.c +++ b/lib/dup2.c @@ -26,19 +26,42 @@ #include <errno.h> #include <fcntl.h> +#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ +/* Get declarations of the Win32 API functions. */ +# define WIN32_LEAN_AND_MEAN +# include <windows.h> +#endif + #if REPLACE_DUP2 -/* On mingw, dup2 exists, but always returns 0 for success. */ + +# undef dup2 + int -dup2 (int fd, int desired_fd) -#undef dup2 +rpl_dup2 (int fd, int desired_fd) { - int result = dup2 (fd, desired_fd); + int result; +# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ + /* If fd is closed, mingw hangs on dup2 (fd, fd). If fd is open, + dup2 (fd, fd) returns 0, but all further attempts to use fd in + future dup2 calls will hang. */ + if (fd == desired_fd) + { + if ((HANDLE) _get_osfhandle (fd) == INVALID_HANDLE_VALUE) + { + errno = EBADF; + return -1; + } + return fd; + } +# endif + result = dup2 (fd, desired_fd); if (result == 0) result = desired_fd; return result; } #else /* !REPLACE_DUP2 */ + /* On older platforms, dup2 did not exist. */ # ifndef F_DUPFD diff --git a/lib/fchdir.c b/lib/fchdir.c index cc2fa63..d51d963 100644 --- a/lib/fchdir.c +++ b/lib/fchdir.c @@ -172,18 +172,19 @@ rpl_dup (int oldfd) return newfd; } +/* Our <unistd.h> replacement overrides dup2 twice; be sure to pick + the one we want. */ +#if REPLACE_DUP2 +# undef dup2 +# define dup2 rpl_dup2 +#endif + int -rpl_dup2 (int oldfd, int newfd) -#undef dup2 +rpl_dup2_fchdir (int oldfd, int newfd) { int retval = dup2 (oldfd, newfd); -#if REPLACE_DUP2 - /* Inline mingw replacement from dup2.c. */ - if (retval == 0) - retval = newfd; -#endif - if (retval >= 0 && oldfd >= 0 && newfd >= 0 && newfd != oldfd) + if (retval >= 0 && newfd != oldfd) { ensure_dirs_slot (newfd); if (newfd < dirs_allocated) diff --git a/lib/unistd.in.h b/lib/unistd.in.h index 47d3096..93edb48 100644 --- a/lib/unistd.in.h +++ b/lib/unistd.in.h @@ -217,10 +217,12 @@ extern int fchdir (int /*fd*/); # define dup rpl_dup extern int dup (int); -# if !...@replace_dup2@ -# define dup2 rpl_dup2 -extern int dup2 (int, int); + +# if @REPLACE_DUP2@ +# undef dup2 # endif +# define dup2 rpl_dup2_fchdir +extern int dup2 (int, int); # endif #elif defined GNULIB_POSIXCHECK -- 1.6.3.2