From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.126.133]) by sourceware.org (Postfix) with ESMTPS id 85BB23858D39 for ; Tue, 26 Oct 2021 16:49:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 85BB23858D39 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=cygwin.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=cygwin.com Received: from calimero.vinschen.de ([24.134.7.25]) by mrelayeu.kundenserver.de (mreue009 [212.227.15.167]) with ESMTPSA (Nemesis) id 1MJV9M-1mLgve04c4-00JpIO for ; Tue, 26 Oct 2021 18:49:34 +0200 Received: by calimero.vinschen.de (Postfix, from userid 500) id 765CDA80DAB; Tue, 26 Oct 2021 18:49:33 +0200 (CEST) Date: Tue, 26 Oct 2021 18:49:33 +0200 From: Corinna Vinschen To: cygwin-developers@cygwin.com Subject: Re: malloc crash Message-ID: Reply-To: cygwin-developers@cygwin.com Mail-Followup-To: cygwin-developers@cygwin.com References: <6a4d6675-7e4d-bcb3-9aff-acc0788d211d@cornell.edu> <97873b16-7ec3-02d7-1861-3ec62a79c37e@cornell.edu> <4b322eb0-4941-6b8f-6f46-aa76caf5a66f@cornell.edu> <66ba46c1-afcd-5f31-58ef-06f96209127e@cornell.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <66ba46c1-afcd-5f31-58ef-06f96209127e@cornell.edu> X-Provags-ID: V03:K1:SrFPgmVIf9Eoc2NJKilTGWUX5SpC1hliLtsydlbpjHsU6kWB+2C 7OppP0+VPrInumrqrgs/NffsJmeQN07JJp0WJ6NM76/baacNugya7Oi33bIslXg4SVhvyZj upmUv0O2SL7uLlm8Dj+ZA7zMsqwVj7M85EGQRpAnoXXPHwz9eGvC22kSWQpJm5L2Hol3Jui LJYsuktSl5LRPQ81x4Omg== X-UI-Out-Filterresults: notjunk:1;V03:K0:4oKolQhqjr4=:xSDEykg5Kh/pycL+aAj7yq e2EP6JTQGnWJmHW3QGtXnxPp6rnU8tFkrtEAiexPmiSOsAxH5AQeSHPBbxqyR/PErUE3c+jdA qXGdVu1WxIGPACZyQC5lc37lDaCfBvLqAKpeb9yThV0o4dd3jHQvircys63LvQmpMlCb9IxrH iNyy4lZ3P6KWvLpbP+XcEchR+9AKLRfuMlcVLTUtkx7ktyXWCPRrhWXo0QiQBUc7szqUdgQEw 9c4+tt/AhqUxglbqhzQcelttDrhXTshG6/AlZhgjbaZY5R84I9V7p2gm52rDX8Rl2QoP2TiB5 YGsG80B1EpC0qCvV2wFpX5jBr/AHoDeHgqF7jjBqHJCU043vtR1Y8HhnH+xtpKnqQM2V5j6Wh YLnyxN8BDt9Q1txx42VlfX+NhrmI2GmMdhgNY66/IprXlkWFPUK6wOIf8EIC4l3LLx0ZNp/Kf 2WEW2p/wsx18rISwcz4sxugOZ/hMQBhOnCUyskfT6c+prJPiLyCOTeDXK4W95ZsM+JijmkZ9V qTWyX5jA7xMPpVGB61FxDAurrfn+kG5K7SNg34NsFhk7T+E0h1wm0522lWVLDinYpUPSMxpD2 tJOhlc9fkZLo3qunu4lUcFVaqt3+Flw96w9Gm8k0bYbRBqyrUlY7T1ARAP5JfusyIwH2PdKWP Kaf882z7Z/+l4bH8KMWFBiUOnAyAI1WNUfYF7pCKUFe8bdI1hbj3bZJUNxN7kiJIKmiFgWgbk 7sUrLjAHoc9zz0FJ X-Spam-Status: No, score=-105.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, GOOD_FROM_CORINNA_CYGWIN, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: cygwin-developers@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin core component developers mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Oct 2021 16:49:37 -0000 On Oct 26 12:36, Ken Brown wrote: > On 10/26/2021 12:03 PM, Corinna Vinschen wrote: > > On Oct 26 10:32, Ken Brown wrote: > > > It might take a while to get this right, and the bug has existed ever since > > > I overhauled the fifo code. So I don't think you have to hold up releasing > > > 3.3.0 while I work on this. > > > > Try the below patch instead, per Takashi's testing and subsequent discussion. > > [...] > I agree with Takashi that this fixes the problem. Thanks to both of you! > This saved me a lot of testing and the risk of introducing new bugs. Below's another incarnation of this patch. I wasn't quite happy with the first one. Statically initialized SRWLOCKs seem to be the natural way to solve the problem... Ken, Takashi, can you please test it both, just to get as much testing as possible? Thanks, Corinna >From 44a79a6eca3d322020dda0919023d78dda129d4d Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Tue, 26 Oct 2021 17:53:08 +0200 Subject: [PATCH] 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 --- 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 84bad824c5ae..36f56294fb35 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 6f4723bb059d..f3d09c1695f8 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 719217856ae3..912f1727441e 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 b839c8cd48ee..f27f81bc4b59 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 565758e4872c..25200f286f8a 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 3b245800abec..2842e5398501 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 895c273974cd..1595b17539ce 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 -- 2.31.1