public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
* [newlib-cygwin] Cygwin: convert malloc lock to SRWLOCK
@ 2021-10-27  9:01 Corinna Vinschen
  0 siblings, 0 replies; only message in thread
From: Corinna Vinschen @ 2021-10-27  9:01 UTC (permalink / raw)
  To: cygwin-cvs

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=44a79a6eca3d322020dda0919023d78dda129d4d

commit 44a79a6eca3d322020dda0919023d78dda129d4d
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Tue Oct 26 17:53:08 2021 +0200

    Cygwin: convert malloc lock to SRWLOCK
    
    Per https://cygwin.com/pipermail/cygwin-developers/2021-October/012429.html,
    we may encounter a crash when starting multiple threads during process
    startup (here: fhandler_fifo::fixup_after_{fork,exec}) which in turn
    allocate memory via malloc.
    
    The problem is concurrent usage of malloc before the malloc muto has
    been initialized.
    
    To fix this issue, convert the muto to a SRWLOCK and make sure it is
    statically initalized.  Thus, malloc can be called as early as necessary
    and malloc_init is only required to check for user space provided malloc.
    
    Note that this requires to implement a __malloc_trylock macro to be
    called from fork.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/cygmalloc.h       | 8 +++++---
 winsup/cygwin/dcrt0.cc          | 1 +
 winsup/cygwin/fork.cc           | 2 +-
 winsup/cygwin/heap.cc           | 1 -
 winsup/cygwin/heap.h            | 1 -
 winsup/cygwin/malloc_wrapper.cc | 4 +---
 winsup/cygwin/release/3.3.0     | 4 ++++
 7 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/winsup/cygwin/cygmalloc.h b/winsup/cygwin/cygmalloc.h
index 84bad824c..36f56294f 100644
--- a/winsup/cygwin/cygmalloc.h
+++ b/winsup/cygwin/cygmalloc.h
@@ -36,9 +36,11 @@ void *mmap64 (void *, size_t, int, int, int, off_t);
 
 #elif defined (__INSIDE_CYGWIN__)
 
-# define __malloc_lock() mallock.acquire ()
-# define __malloc_unlock() mallock.release ()
-extern muto mallock;
+# define __malloc_lock() AcquireSRWLockExclusive (&mallock)
+# define __malloc_trylock() TryAcquireSRWLockExclusive (&mallock)
+# define __malloc_unlock() ReleaseSRWLockExclusive (&mallock)
+extern SRWLOCK NO_COPY mallock;
+void malloc_init ();
 
 #endif
 
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 6f4723bb0..f3d09c169 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -29,6 +29,7 @@ details. */
 #include "shared_info.h"
 #include "cygwin_version.h"
 #include "dll_init.h"
+#include "cygmalloc.h"
 #include "heap.h"
 #include "tls_pbuf.h"
 #include "exception.h"
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 719217856..912f17274 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -296,7 +296,7 @@ frok::parent (volatile char * volatile stack_here)
   si.lpReserved2 = (LPBYTE) &ch;
   si.cbReserved2 = sizeof (ch);
 
-  bool locked = __malloc_lock ();
+  bool locked = __malloc_trylock ();
 
   /* Remove impersonation */
   cygheap->user.deimpersonate ();
diff --git a/winsup/cygwin/heap.cc b/winsup/cygwin/heap.cc
index b839c8cd4..f27f81bc4 100644
--- a/winsup/cygwin/heap.cc
+++ b/winsup/cygwin/heap.cc
@@ -230,7 +230,6 @@ user_heap_info::init ()
   debug_printf ("heap base %p, heap top %p, heap size %ly (%lu)",
 		base, top, chunk, chunk);
   page_const--;
-  // malloc_init ();
 }
 
 #define pround(n) (((size_t)(n) + page_const) & ~page_const)
diff --git a/winsup/cygwin/heap.h b/winsup/cygwin/heap.h
index 565758e48..25200f286 100644
--- a/winsup/cygwin/heap.h
+++ b/winsup/cygwin/heap.h
@@ -10,7 +10,6 @@ details. */
 
 /* Heap management. */
 void heap_init ();
-void malloc_init ();
 
 #define inheap(s) \
   (cygheap->user_heap.ptr && s \
diff --git a/winsup/cygwin/malloc_wrapper.cc b/winsup/cygwin/malloc_wrapper.cc
index 3b245800a..2842e5398 100644
--- a/winsup/cygwin/malloc_wrapper.cc
+++ b/winsup/cygwin/malloc_wrapper.cc
@@ -269,13 +269,11 @@ strdup (const char *s)
    newlib will call __malloc_lock and __malloc_unlock at appropriate
    times.  */
 
-muto NO_COPY mallock;
+SRWLOCK NO_COPY mallock = SRWLOCK_INIT;
 
 void
 malloc_init ()
 {
-  mallock.init ("mallock");
-
   /* Check if malloc is provided by application. If so, redirect all
      calls to malloc/free/realloc to application provided. This may
      happen if some other dll calls cygwin's malloc, but main code provides
diff --git a/winsup/cygwin/release/3.3.0 b/winsup/cygwin/release/3.3.0
index 895c27397..1595b1753 100644
--- a/winsup/cygwin/release/3.3.0
+++ b/winsup/cygwin/release/3.3.0
@@ -78,3 +78,7 @@ Bug Fixes
 - Fix access violation that can sometimes occur when copy/pasting between
   32-bit and 64-bit Cygwin environments.  Align clipboard descriptor layouts.
   Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q4/011517.html
+
+- Fix a synchronization issue when running multiple threads from DLL
+  initialization which in turn call malloc.
+  Addresses: https://cygwin.com/pipermail/cygwin/2021-October/249635.html


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-10-27  9:01 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-27  9:01 [newlib-cygwin] Cygwin: convert malloc lock to SRWLOCK Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).