From 0dc723957d0fdb5909f145405b775efea0fe2f6e Mon Sep 17 00:00:00 2001
From: Tamar Christina <tamar@zhox.com>
Date: Tue, 28 Dec 2021 15:06:16 +0000
Subject: [PATCH] winio: fix heap corruption and various leaks.
---
libraries/base/GHC/Event/Windows.hsc | 5 ++++-
libraries/base/GHC/IO/Windows/Paths.hs | 2 +-
libraries/base/System/IO.hs | 24 ++++++++++++------------
libraries/base/cbits/Win32Utils.c | 4 ++--
rts/RtsStartup.c | 3 +++
rts/RtsSymbols.c | 3 ++-
rts/include/rts/IOInterface.h | 1 +
rts/win32/AsyncWinIO.c | 3 ++-
rts/win32/ConsoleHandler.c | 2 +-
rts/win32/ConsoleHandler.h | 7 +++++++
rts/win32/ThrIOManager.c | 10 ++++++++++
rts/win32/ThrIOManager.h | 2 +-
12 files changed, 46 insertions(+), 20 deletions(-)
diff --git a/libraries/base/GHC/Event/Windows.hsc b/libraries/base/GHC/Event/Windows.hsc
index b97250e897d3..cc6bbaa927de 100644
--- a/libraries/base/GHC/Event/Windows.hsc
+++ b/libraries/base/GHC/Event/Windows.hsc
@@ -1162,7 +1162,7 @@ io_mngr_loop _event mgr = go False
exit <-
case event_id of
_ | event_id == io_MANAGER_WAKEUP -> return False
- _ | event_id == io_MANAGER_DIE -> return True
+ _ | event_id == io_MANAGER_DIE -> c_ioManagerFinished >> return True
0 -> return False -- spurious wakeup
_ -> do debugIO $ "handling console event: " ++ show (event_id `shiftR` 1)
start_console_handler (event_id `shiftR` 1)
@@ -1203,6 +1203,9 @@ foreign import ccall unsafe "getIOManagerEvent" -- in the RTS (ThrIOManager.c)
foreign import ccall unsafe "readIOManagerEvent" -- in the RTS (ThrIOManager.c)
c_readIOManagerEvent :: IO Word32
+foreign import ccall unsafe "ioManagerFinished" -- in the RTS (ThrIOManager.c)
+ c_ioManagerFinished :: IO ()
+
foreign import ccall unsafe "rtsSupportsBoundThreads" threadedIOMgr :: Bool
-- | Sleep for n ms
diff --git a/libraries/base/GHC/IO/Windows/Paths.hs b/libraries/base/GHC/IO/Windows/Paths.hs
index 851dc3750811..c755996f220b 100644
--- a/libraries/base/GHC/IO/Windows/Paths.hs
+++ b/libraries/base/GHC/IO/Windows/Paths.hs
@@ -30,7 +30,7 @@ import GHC.IO
import Foreign.C.String
import Foreign.Marshal.Alloc (free)
-foreign import WINDOWS_CCONV safe "__hs_create_device_name"
+foreign import ccall safe "__hs_create_device_name"
c_GetDevicePath :: CWString -> IO CWString
-- | This function converts Windows paths between namespaces. More specifically
diff --git a/libraries/base/System/IO.hs b/libraries/base/System/IO.hs
index 265507d97085..f831df6cb4fd 100644
--- a/libraries/base/System/IO.hs
+++ b/libraries/base/System/IO.hs
@@ -231,13 +231,13 @@ import Foreign.C.Error
import Foreign.C.String
import Foreign.Ptr
import Foreign.Marshal.Alloc
+import Foreign.Marshal.Utils (with)
import Foreign.Storable
import GHC.IO.SubSystem
import GHC.IO.Windows.Handle (openFileAsTemp)
import GHC.IO.Handle.Windows (mkHandleFromHANDLE)
import GHC.IO.Device as IODevice
import GHC.Real (fromIntegral)
-import Foreign.Marshal.Utils (new)
#endif
import Foreign.C.Types
import System.Posix.Internals
@@ -529,17 +529,17 @@ openTempFile' loc tmp_dir template binary mode
let label = if null prefix then "ghc" else prefix
withCWString tmp_dir $ \c_tmp_dir ->
withCWString label $ \c_template ->
- withCWString suffix $ \c_suffix -> do
- c_ptr <- new nullPtr
- res <- c_createUUIDTempFileErrNo c_tmp_dir c_template c_suffix
- c_ptr
- if not res
- then do errno <- getErrno
- ioError (errnoToIOError loc errno Nothing (Just tmp_dir))
- else do c_p <- peek c_ptr
- filename <- peekCWString c_p
- free c_p
- handleResultsWinIO filename ((fromIntegral mode .&. o_EXCL) == o_EXCL)
+ withCWString suffix $ \c_suffix ->
+ with nullPtr $ \c_ptr -> do
+ res <- c_createUUIDTempFileErrNo c_tmp_dir c_template c_suffix c_ptr
+ if not res
+ then do errno <- getErrno
+ ioError (errnoToIOError loc errno Nothing (Just tmp_dir))
+ else do c_p <- peek c_ptr
+ filename <- peekCWString c_p
+ free c_p
+ let flags = fromIntegral mode .&. o_EXCL
+ handleResultsWinIO filename (flags == o_EXCL)
findTempNamePosix = do
let label = if null prefix then "ghc" else prefix
diff --git a/libraries/base/cbits/Win32Utils.c b/libraries/base/cbits/Win32Utils.c
index f3dec0d98dd2..69d30339ba57 100644
--- a/libraries/base/cbits/Win32Utils.c
+++ b/libraries/base/cbits/Win32Utils.c
@@ -183,10 +183,9 @@ bool __createUUIDTempFileErrNo (wchar_t* pathName, wchar_t* prefix,
RPC_WSTR guidStr;
if (UuidToStringW ((UUID*)&guid, &guidStr) != S_OK)
goto fail;
-
/* We can't create a device path here since this path escapes the compiler
so instead return a normal path and have openFile deal with it. */
- wchar_t* devName = malloc (sizeof (wchar_t) * wcslen (pathName));
+ wchar_t* devName = malloc (sizeof (wchar_t) * (wcslen (pathName) + 1));
wcscpy (devName, pathName);
int len = wcslen (devName) + wcslen (suffix) + wcslen (prefix)
+ wcslen (guidStr) + 3;
@@ -204,6 +203,7 @@ bool __createUUIDTempFileErrNo (wchar_t* pathName, wchar_t* prefix,
free (devName);
RpcStringFreeW (&guidStr);
+
/* This should never happen because GUIDs are unique. But in case hell
froze over let's check anyway. */
DWORD dwAttrib = GetFileAttributesW (*tempFileName);
diff --git a/rts/RtsStartup.c b/rts/RtsStartup.c
index 86d5b2f2d96e..07db7b7998b3 100644
--- a/rts/RtsStartup.c
+++ b/rts/RtsStartup.c
@@ -578,6 +578,9 @@ hs_exit_(bool wait_foreign)
#if defined(mingw32_HOST_OS)
if (is_io_mng_native_p())
hs_restoreConsoleCP();
+
+ /* Disable console signal handlers, we're going down!. */
+ finiUserSignals ();
#endif
/* tear down statistics subsystem */
diff --git a/rts/RtsSymbols.c b/rts/RtsSymbols.c
index 894e9ff30578..b6440049c091 100644
--- a/rts/RtsSymbols.c
+++ b/rts/RtsSymbols.c
@@ -375,13 +375,14 @@ extern char **environ;
SymI_HasProto(unblockUserSignals)
#else
#define RTS_USER_SIGNALS_SYMBOLS \
- SymI_HasProto(registerIOCPHandle) \
+ SymI_HasProto(registerIOCPHandle) \
SymI_HasProto(getOverlappedEntries) \
SymI_HasProto(completeSynchronousRequest) \
SymI_HasProto(registerAlertableWait) \
SymI_HasProto(sendIOManagerEvent) \
SymI_HasProto(readIOManagerEvent) \
SymI_HasProto(getIOManagerEvent) \
+ SymI_HasProto(ioManagerFinished) \
SymI_HasProto(console_handler)
#endif
diff --git a/rts/include/rts/IOInterface.h b/rts/include/rts/IOInterface.h
index 9a646cc5cf2b..fd625eda70ba 100644
--- a/rts/include/rts/IOInterface.h
+++ b/rts/include/rts/IOInterface.h
@@ -27,6 +27,7 @@ extern StgInt console_handler;
void * getIOManagerEvent (void);
HsWord32 readIOManagerEvent (void);
void sendIOManagerEvent (HsWord32 event);
+void ioManagerFinished (void);
#else
diff --git a/rts/win32/AsyncWinIO.c b/rts/win32/AsyncWinIO.c
index 7fb71e92e740..5f61f815f382 100644
--- a/rts/win32/AsyncWinIO.c
+++ b/rts/win32/AsyncWinIO.c
@@ -147,7 +147,7 @@
There we initialize IO manager locale variables and.
* call ioManagerStart()
- * Creat a thread to execute "runner"
+ * Create a thread to execute "runner"
We never truly shut down the IO Manager. While this means we
might block forever on the IOPort if the IO Manager is no longer
@@ -274,6 +274,7 @@ void shutdownAsyncWinIO(bool wait_threads)
WaitForSingleObject (workerThread, INFINITE);
}
completionPortHandle = INVALID_HANDLE_VALUE;
+ CloseHandle (workerThread);
workerThread = NULL;
workerThreadId = 0;
free (entries);
diff --git a/rts/win32/ConsoleHandler.c b/rts/win32/ConsoleHandler.c
index 05d15868eb1b..4af897bc54b5 100644
--- a/rts/win32/ConsoleHandler.c
+++ b/rts/win32/ConsoleHandler.c
@@ -57,7 +57,7 @@ freeSignalHandlers(void) {
/* Do nothing */
}
-/* Seems to be a bit of an orphan...where used? */
+/* Called in hs_exit to clean up resources. */
void
finiUserSignals(void)
{
diff --git a/rts/win32/ConsoleHandler.h b/rts/win32/ConsoleHandler.h
index bb7278abbab4..d22f10f16a8e 100644
--- a/rts/win32/ConsoleHandler.h
+++ b/rts/win32/ConsoleHandler.h
@@ -62,3 +62,10 @@ extern void startSignalHandlers(Capability *cap);
extern int rts_waitConsoleHandlerCompletion(void);
#endif /* THREADED_RTS */
+
+/*
+ * Function: finiUserSignals()
+ *
+ * Tear down and shut down user signal processing.
+ */
+extern void finiUserSignals(void);
diff --git a/rts/win32/ThrIOManager.c b/rts/win32/ThrIOManager.c
index d614d49a0c9b..6bbf65a45f51 100644
--- a/rts/win32/ThrIOManager.c
+++ b/rts/win32/ThrIOManager.c
@@ -161,3 +161,13 @@ ioManagerStart (void)
rts_unlock(cap);
}
}
+
+/*
+ * Called to close the io_manager_event handle when the IO manager thread is
+ * terminating.
+ */
+void
+ioManagerFinished(void)
+{
+ CloseHandle(io_manager_event);
+}
diff --git a/rts/win32/ThrIOManager.h b/rts/win32/ThrIOManager.h
index a67bdde3648e..c7ecc76e9fdf 100644
--- a/rts/win32/ThrIOManager.h
+++ b/rts/win32/ThrIOManager.h
@@ -15,4 +15,4 @@
void ioManagerWakeup (void);
void ioManagerDie (void);
void ioManagerStart (void);
-
+void ioManagerFinished (void);
--
GitLab