public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Corinna Vinschen <corinna@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin] Cygwin: convert malloc lock to SRWLOCK
Date: Wed, 27 Oct 2021 09:01:03 +0000 (GMT)	[thread overview]
Message-ID: <20211027090103.71A513858C27@sourceware.org> (raw)

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


                 reply	other threads:[~2021-10-27  9:01 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211027090103.71A513858C27@sourceware.org \
    --to=corinna@sourceware.org \
    --cc=cygwin-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).