From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.17.10]) by sourceware.org (Postfix) with ESMTPS id 5554F3858D39 for ; Tue, 26 Oct 2021 16:03:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5554F3858D39 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 (mreue108 [212.227.15.183]) with ESMTPSA (Nemesis) id 1N63JO-1mhnWS47nU-016MW2 for ; Tue, 26 Oct 2021 18:03:29 +0200 Received: by calimero.vinschen.de (Postfix, from userid 500) id 09674A80DAB; Tue, 26 Oct 2021 18:03:28 +0200 (CEST) Date: Tue, 26 Oct 2021 18:03:28 +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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Provags-ID: V03:K1:HbJ2RSifM16MH0l2hBTJdIth28869lVGwERNAi7g8E0MziPbLlz MKM29PNvgWYtWQSCK5teaWR8QWD0fiDgtAkJZPGBkLihuoQrN9tB42kwaev5yGNDm+XmFyt EK0Bmk6Aw5HinvhtV2CXVBeSD3xzPIWQtfmHoKSEzatW4Tj5Bw8isg8Q2Fq0mMhps/tMxnj YZB3BJjPb6Eqpa7kgRqGQ== X-UI-Out-Filterresults: notjunk:1;V03:K0:6wwaeWT766A=:HKEfwIR/9Wf51LEVNb2i4L FJr/n41oX+LKbM+E/K/nfev0TlMpONJpcli6qftfZ1OBNfnIeIscCA3x15FOEyt2oqH/X79DW n+s4fOSM93K38+7g008WWe1a45VIec9Q8OsjB+ufr2BRwgHVrbDhBDSkvsW6zbmGrnnVZww72 7lcic9DwVni/SQW1mccrNj+5gJ05V+Djz8CUXqkwIfcrReL9CWFtMlz7y0pyT3a5u9F54C+DA JdH9UMAJi5PAAs/gFmPKyIna6PhYBG+6W0rp2B/E5MPybRVOG+lVCWNfToEddvP9DB0KDJXkO Uncg7ZIZhQ8njN0m7I9USbUqLMYvaP0+3P/MhsO/EwlQyNZkAEnbvQ44ZWOh8i+1Nlf/1SwcI y4aa1s+aXt7aGyU7vVmOjE45Dj+C9BkJQTHDnse/W8diQJAlqTES/GZLlLs42AJCFw2T2WMsf 0Erww+DPER3r5MmNn8XvkZDIZMZPu58HBsozdyG2w0eaKG+eU9OkB6GZG35NWgzUywxfUp8tI Am4L7DBlrgpTr3hJcmcd7e0Ewq2lyAk6rEgCfMR1RMj4L1P0eUPkImb9/t8Pq1BXPcPF9K3Ix BzsDQbVYi7GPxb2LTant9IZFePt0K3CqE5qzTp0Lb38BKXjx5Fsb4iUht7BPWx4XKmkaLpEcS nuaxUcdRumGKXZzU0+WIEonFRtjaRhPGgGxkoLuoc/Ge4btXBQBRe9MNWAkFtbTq6GuPsOJyj hGGTDq+YnXkb07mo 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_MSPIKE_H2, 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:03:33 -0000 On Oct 26 10:32, Ken Brown wrote: > On 10/26/2021 5:24 AM, Corinna Vinschen wrote: > > On Oct 25 18:02, Ken Brown wrote: > > > Or does the fifo_reader thread call a malloc function before the main thread > > > has called malloc_init()? This would presumably cause __malloc_lock() to > > > fail, but there's no error check. > > > > That sounds more likely. In theory this shouldn't have much influence, > > though. First of all, all fixup calls are running in a single thread, > > so there's no serialization required(*), and the malloc_init call > > doesn't set up the malloc arena, it just initializes the muto and checks > > for user space provided malloc calls, which is not a problem in this > > scenario. > > > > (*) unless multiple threads are started during fixup and some of these > > threads mallocate memory again... > > > > Ken, is there a chance to tweak the fifo code to stop creating > > threads from inside fixup, and to defer the thread start to the first > > call in the process actually relying on the thread being started? > > I can't think of any way to do that. The thread is listening for various > events that cause it to take action, so it has to always be running. But I > can probably tweak the code so that the thread doesn't have to call malloc > early on. > > 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. >From 9e53881e81bc6d2d072a0d625a9eac8ffc7cc698 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Tue, 26 Oct 2021 17:53:08 +0200 Subject: [PATCH] Cygwin: split malloc_init 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, split malloc_init into malloc_init_0, called from dll_crt0_0, and malloc_init_1, called from dll_crt_0_1. malloc_init_0 just initializes the muto, malloc_init_1 checks for user space provided malloc. Signed-off-by: Corinna Vinschen --- winsup/cygwin/dcrt0.cc | 4 +++- winsup/cygwin/heap.cc | 1 - winsup/cygwin/heap.h | 3 ++- winsup/cygwin/malloc_wrapper.cc | 6 +++++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc index 6f4723bb059d..7c460274fb86 100644 --- a/winsup/cygwin/dcrt0.cc +++ b/winsup/cygwin/dcrt0.cc @@ -769,6 +769,8 @@ dll_crt0_0 () NtOpenProcessToken (NtCurrentProcess (), MAXIMUM_ALLOWED, &hProcToken); set_cygwin_privileges (hProcToken); + malloc_init_0 (); + device::init (); do_global_ctors (&__CTOR_LIST__, 1); cygthread::init (); @@ -857,7 +859,7 @@ dll_crt0_1 (void *) on a functioning malloc and it's possible that the user's program may have overridden malloc. We only know about that at this stage, unfortunately. */ - malloc_init (); + malloc_init_1 (); user_shared->initialize (); #ifdef CYGHEAP_DEBUG 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..42099051f619 100644 --- a/winsup/cygwin/heap.h +++ b/winsup/cygwin/heap.h @@ -10,7 +10,8 @@ details. */ /* Heap management. */ void heap_init (); -void malloc_init (); +void malloc_init_0 (); +void malloc_init_1 (); #define inheap(s) \ (cygheap->user_heap.ptr && s \ diff --git a/winsup/cygwin/malloc_wrapper.cc b/winsup/cygwin/malloc_wrapper.cc index 3b245800abec..85c411a3e258 100644 --- a/winsup/cygwin/malloc_wrapper.cc +++ b/winsup/cygwin/malloc_wrapper.cc @@ -272,10 +272,14 @@ strdup (const char *s) muto NO_COPY mallock; void -malloc_init () +malloc_init_0 () { mallock.init ("mallock"); +} +void +malloc_init_1 () +{ /* 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 -- 2.31.1