diff --git a/System/Process.hs b/System/Process.hs --- a/System/Process.hs +++ b/System/Process.hs @@ -20,7 +20,7 @@ -- ----------------------------------------------------------------------------- --- ToDo: +-- TODO: -- * Flag to control whether exiting the parent also kills the child. module System.Process ( @@ -383,7 +383,7 @@ -- @SIGINT@ to every process using the console. The standard solution is that -- while running an interactive program, ignore @SIGINT@ in the parent, and let -- it be handled in the child process. If that process then terminates due to --- the @SIGINT@ signal, then at that point treat it as if we had recieved the +-- the @SIGINT@ signal, then at that point treat it as if we had received the -- @SIGINT@ ourselves and begin an orderly shutdown. -- -- This behaviour is implemented by 'createProcess' (and diff --git a/System/Process/Common.hs b/System/Process/Common.hs --- a/System/Process/Common.hs +++ b/System/Process/Common.hs @@ -292,6 +292,7 @@ do raw_handle <- peek pfd let hwnd = fromHANDLE raw_handle :: Io NativeHandle ident = "hwnd:" ++ show raw_handle - Just <$> mkHandleFromHANDLE hwnd Stream ident mode Nothing + enc <- fmap Just getLocaleEncoding + Just <$> mkHandleFromHANDLE hwnd Stream ident mode enc mbPipeHANDLE _std _pfd _mode = return Nothing #endif diff --git a/System/Process/Internals.hs b/System/Process/Internals.hs --- a/System/Process/Internals.hs +++ b/System/Process/Internals.hs @@ -182,6 +182,33 @@ -- | Create a pipe for interprocess communication and return a -- @(readEnd, writeEnd)@ `Handle` pair. -- +-- * WinIO Support +-- +-- When this function is used with WinIO enabled it's the caller's +-- responsibility to register the handles with the I/O manager. +-- If this is not done the operation will deadlock. Association can +-- be done as follows: +-- +-- @ +-- #if defined(__IO_MANAGER_WINIO__) +-- import GHC.IO.SubSystem (()) +-- import GHC.IO.Handle.Windows (handleToHANDLE) +-- import GHC.Event.Windows (associateHandle') +-- #endif +-- +-- ... +-- +-- #if defined (__IO_MANAGER_WINIO__) +-- return () (do +-- associateHandle' =<< handleToHANDLE ) +-- #endif +-- @ +-- +-- Only associate handles that you are in charge of read/writing to. +-- Do not associate handles passed to another process. It's the +-- process's reponsibility to register the handle if it supports +-- async access. +-- -- @since 1.2.1.0 createPipe :: IO (Handle, Handle) createPipe = createPipeInternal diff --git a/System/Process/Windows.hsc b/System/Process/Windows.hsc --- a/System/Process/Windows.hsc +++ b/System/Process/Windows.hsc @@ -450,8 +450,8 @@ alloca $ \ pfdStdOutput -> do throwErrnoIf_ (==False) "c_mkNamedPipe" $ c_mkNamedPipe pfdStdInput True pfdStdOutput True - Just hndStdInput <- mbPipeHANDLE CreatePipe pfdStdInput WriteMode - Just hndStdOutput <- mbPipeHANDLE CreatePipe pfdStdOutput ReadMode + Just hndStdInput <- mbPipeHANDLE CreatePipe pfdStdInput ReadMode + Just hndStdOutput <- mbPipeHANDLE CreatePipe pfdStdOutput WriteMode return (hndStdInput, hndStdOutput) diff --git a/aclocal.m4 b/aclocal.m4 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -8,7 +8,7 @@ # is supported in autoconf versions 2.50 up to the actual 2.57, so there is # little risk. AC_DEFUN([FP_COMPUTE_INT], -[_AC_COMPUTE_INT([$1], [$2], [$3], [$4])[]dnl +[AC_COMPUTE_INT([$2],[$1],[$3],[$4])[]dnl ])# FP_COMPUTE_INT @@ -31,8 +31,7 @@ # --------------------------------------- # autoheader helper for FP_CHECK_CONSTS m4_define([FP_CHECK_CONSTS_TEMPLATE], -[AC_FOREACH([fp_Const], [$1], - [AH_TEMPLATE(AS_TR_CPP(CONST_[]fp_Const), +[m4_foreach_w([fp_Const],[$1],[AH_TEMPLATE(AS_TR_CPP(CONST_[]fp_Const), [The value of ]fp_Const[.])])[]dnl ])# FP_CHECK_CONSTS_TEMPLATE diff --git a/cbits/posix/fork_exec.c b/cbits/posix/fork_exec.c --- a/cbits/posix/fork_exec.c +++ b/cbits/posix/fork_exec.c @@ -69,8 +69,13 @@ return 0; case STD_HANDLE_USE_FD: - if (dup2(b->use_fd, fd) == -1) { - child_failed(pipe, "dup2"); + // N.B. POSIX specifies that dup2(x,x) should be a no-op, but + // naturally Apple ignores this and rather fails in posix_spawn on Big + // Sur. + if (b->use_fd != fd) { + if (dup2(b->use_fd, fd) == -1) { + child_failed(pipe, "dup2"); + } } return 0; diff --git a/cbits/win32/runProcess.c b/cbits/win32/runProcess.c --- a/cbits/win32/runProcess.c +++ b/cbits/win32/runProcess.c @@ -123,11 +123,30 @@ /* Create one end of the pipe. Named pipes are a bit less secure than anonymous pipes. Because of this we restrict the pipe's access to only one client and also only the local host. This means after we create the - other end of the pipe it should be as secure as an anonymous pipe. */ + other end of the pipe it should be as secure as an anonymous pipe. + + "When you operate on named pipes, you have a choice of opening them in + PIPE_WAIT mode or PIPE_NOWAIT mode. When you read from a PIPE_WAIT pipe, + the read blocks until data becomes available in the pipe. When you read + from a PIPE_NOWAIT pipe, then the read completes immediately even if + there is no data in the pipe. But how is this different from a PIPE_WAIT + pipe opened in asynchronous mode by passing FILE_FLAG_OVERLAPPED? The + difference is in when the I/O is deemed to have completed. When you issue + an overlapped read against a PIPE_WAIT pipe, the call to Read­File returns + immediately, but the completion actions do not occur until there is data + available in the pipe. (Completion actions are things like setting the + event, running the completion routine, or queueing a completion to an I/O + completion port.) On the other hand, when you issue a read against a + PIPE_NOWAIT pipe, the call to Read­File returns immediately with + completion—if the pipe is empty, the read completes with a read of zero + bytes and the error ERROR_NO_DATA."[0] + + [0] https://devblogs.microsoft.com/oldnewthing/20110114-00/?p=11753 */ + DWORD inAttr = isInheritableIn ? 0 : FILE_FLAG_OVERLAPPED; hTemporaryIn - = CreateNamedPipeW (&pipeName[0], - PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED | FILE_FLAG_FIRST_PIPE_INSTANCE, - PIPE_TYPE_MESSAGE | PIPE_REJECT_REMOTE_CLIENTS, + = CreateNamedPipeW (pipeName, + PIPE_ACCESS_INBOUND | inAttr | FILE_FLAG_FIRST_PIPE_INSTANCE, + PIPE_TYPE_MESSAGE | PIPE_REJECT_REMOTE_CLIENTS | PIPE_READMODE_MESSAGE | PIPE_WAIT, 1, buffer_size, buffer_size, 0, &secAttr); @@ -138,14 +157,28 @@ will give us the read and write ends of the pipe. */ secAttr.bInheritHandle = isInheritableOut; hTemporaryOut - = CreateFileW (&pipeName[0], + = CreateFileW (pipeName, GENERIC_WRITE, FILE_SHARE_WRITE, &secAttr, OPEN_EXISTING, - FILE_FLAG_OVERLAPPED, + isInheritableOut + ? FILE_ATTRIBUTE_NORMAL + : FILE_FLAG_OVERLAPPED, NULL); + if (hTemporaryOut == INVALID_HANDLE_VALUE) + goto fail; + + /* Ensure that read and write ends are set to the same mode. MESSAGE mode + will honor data boundaries as a whole. That is, if n bytes are posted + at once, n bytes are received together at the other wide as long as n + is smaller than buffer size. Otherwise it's up to buffer size. BYTE + mode is essentially streaming mode. Typically Haskell would benefit from + both modes, cabal from byte streaming and iserv from message. Let's + default to MESSAGE. */ + DWORD pipeFlags = PIPE_READMODE_MESSAGE; + if (!SetNamedPipeHandleState (hTemporaryOut, &pipeFlags, NULL, NULL)) goto fail; /* Set some optimization flags to make the I/O manager operate more diff --git a/changelog.md b/changelog.md --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,13 @@ # Changelog for [`process` package](http://hackage.haskell.org/package/process) +## 1.6.15.0 *August 2022* + +* Correct permissions on createPipe on Windows [234](https://github.com/haskell/process/pull/234) +* Ensure that both ends of pipes on Windows are created in the same mode [234](https://github.com/haskell/process/pull/234) +* Fixed an issue with WINIO where giving an application an inherited pipe can cause it to misbehave [245](https://github.com/haskell/process/pull/245) +* Set the encoding on WINIO created pipes to the local encoding as with MIO [248](https://github.com/haskell/process/pull/248) +* cbits/fork-exec: Don't dup2 identical fds [#250](https://github.com/haskell/process/pull/250) + ## 1.6.14.0 *February 2022* * posix: Ensure that `errno` is set after `posix_spawnp` fails [#228](https://github.com/haskell/process/pull/228) diff --git a/configure.ac b/configure.ac --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([Haskell process package], [1.0], [libraries@haskell.org], [process]) +AC_INIT([Haskell process package],[1.0],[libraries@haskell.org],[process]) # Safety check: Ensure that we are in the correct source directory. AC_CONFIG_SRCDIR([include/runProcess.h]) @@ -59,7 +59,9 @@ )], [AC_DEFINE([USE_POSIX_SPAWN], [], [posix_spawn should be used]) AC_MSG_RESULT(yes)], - [AC_MSG_RESULT([no, falling back to fork/exec])] + [AC_MSG_RESULT([no, falling back to fork/exec])], + [AC_DEFINE([USE_POSIX_SPAWN], [], []) + AC_MSG_RESULT(cross-compiling - assuming yes)] ) fi diff --git a/process.cabal b/process.cabal --- a/process.cabal +++ b/process.cabal @@ -1,5 +1,5 @@ name: process -version: 1.6.14.0 +version: 1.6.15.0 -- NOTE: Don't forget to update ./changelog.md license: BSD3 license-file: LICENSE @@ -58,7 +58,7 @@ c-sources: cbits/win32/runProcess.c other-modules: System.Process.Windows - build-depends: Win32 >=2.4 && < 2.13 + build-depends: Win32 >=2.4 && < 2.14 -- ole32 and rpcrt4 are needed to create GUIDs for unique named pipes -- for process. extra-libraries: kernel32, ole32, rpcrt4 @@ -81,7 +81,7 @@ ghc-options: -Wall - build-depends: base >= 4.10 && < 4.17, + build-depends: base >= 4.10 && < 4.18, directory >= 1.1 && < 1.4, filepath >= 1.2 && < 1.5, deepseq >= 1.1 && < 1.5