From: Carlo Arenas <email@example.com> To: Johannes Schindelin <Johannes.Schindelin@gmx.de> Cc: firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org Subject: Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator Date: Tue, 27 Aug 2019 04:51:14 -0700 [thread overview] Message-ID: <CAPUEspjJNSrJQT7xV2fsdp2t5odW5fzzPdDxuar_5x_JPUtf6Q@mail.gmail.com> (raw) In-Reply-To: <nycvar.QRO.email@example.com> On Tue, Aug 27, 2019 at 2:07 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Unfortunately, this is _still_ incorrect. I know, and that is why we worked out a v6 RC with Rene than then was pushed to github and validated to work thanks to your integration as detailed in , I never got to send an update to the list though because Rene wanted my squashing into his patch to be independent as detailed there and because I assumed that when Junio didn't pull my reroll, it was probably because your tree had a fix already anyway. the recent discovery that xmalloc wasn't thread safe though, complicates things further and that is why I was expecting to reroll this on top of both ab/pcre-jit-fixes and jk/drop-release-pack-memory (later one already in next) as detailed in  > I pointed out multiple times that custom allocators can be activated at > run-time rather than compile-time, therefore making the choice at > compile-time is wrong. Besides, there is nothing specific to nedmalloc > about this. So the patch is double-wrong on that account. Just to clarify, I think my patch accounts for that (haven't tested that assumption, but will do now that I have a windows box, probably even with mi-alloc) but yes, the only reason why there were references to NEDMALLOC was to isolate the code and make sure the fix was tackling the problem, it was not my intention to do so at the end, specially once we agreed that xmalloc should be used anyway. > The patch has a yet even more immediate problem: t7816.48 is failing in > the CI build for _weeks_ now: it requires that global context to be > initialized, but no code path hits the initialization, resulting in a > very, very ugly: > > BUG: grep.c:516: pcre2_global_context uninitialized > > See > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=15151&view=ms.vss-test-web.build-test-results-tab&runId=41282&resultId=101710&paneView=debug > for details. IMHO this is probably testing V3 (from pu) and which was hopefully fixed in the last github force push for my branch > All of this could be easily avoided. As I had pointed out already, the > obvious fragility is not worth the optimization, and we should just > initialize the global context always, as does this patch > (https://github.com/git-for-windows/git/commit/5e5b959169e6efee73e0b50e464166822b7d2d07). ironically only found out about that patch after I got a windows box (running Windows Home though) and had finished testing my own squashed fix that has succeeded being validated in GitHub apologize for the delays, and will be fine using your squash, mine, the V6 RC (my preference) or dropping this series from pu if that would help clear the ugliness of pu for windows hopefully this won't be repeated now that I am aware of github's integration and have my own (albeit very slow) windows environment as well. Carlo  https://github.com/git/git/commit/0ca5d0550c17a68d83b8922b71aeff891958ed0e  https://public-inbox.org/git/CAPUEspiFuvgMQ3W1se1B=8aTTrQsJSZTyQTzG1TpiyN8HTOpkA@mail.gmail.com/  https://public-inbox.org/git/CAPUEspg9F7RutCUCoRAAXmRePjiunq3-zG7cN3uz_t5DVMxPfirstname.lastname@example.org/  https://github.com/git/git/pull/627
next prev parent reply other threads:[~2019-08-27 11:51 UTC|newest] Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-05 11:51 [PATCH 0/1] Fix a problem with PCRE2 and nedmalloc, found via Azure Pipelines Johannes Schindelin via GitGitGadget 2019-08-05 11:51 ` [PATCH 1/1] pcre2: allow overriding the system allocator Johannes Schindelin via GitGitGadget 2019-08-05 16:19 ` Carlo Arenas 2019-08-05 16:27 ` Carlo Arenas 2019-08-05 19:32 ` Johannes Schindelin 2019-08-05 19:26 ` Johannes Schindelin 2019-08-05 21:53 ` Junio C Hamano 2019-08-06 6:24 ` Carlo Arenas 2019-08-06 8:50 ` [PATCH 0/3] grep: no leaks (WIP) Carlo Marcelo Arenas Belón 2019-08-06 8:50 ` [PATCH 1/3] grep: make PCRE1 aware of custom allocator Carlo Marcelo Arenas Belón 2019-08-08 13:54 ` Johannes Schindelin 2019-08-08 15:19 ` Carlo Arenas 2019-08-06 8:50 ` [PATCH 2/3] grep: make PCRE2 " Carlo Marcelo Arenas Belón 2019-08-08 13:56 ` Johannes Schindelin 2019-08-08 14:32 ` Carlo Arenas 2019-08-06 8:50 ` [PATCH 3/3] grep: avoid leak of chartables in PCRE2 Carlo Marcelo Arenas Belón 2019-08-06 16:36 ` [RFC PATCH v3 0/3] grep: no leaks or crashes (windows testing needed) Carlo Marcelo Arenas Belón 2019-08-06 16:36 ` [RFC PATCH v3 1/3] grep: make PCRE1 aware of custom allocator Carlo Marcelo Arenas Belón 2019-08-06 16:36 ` [RFC PATCH v3 2/3] grep: make PCRE2 " Carlo Marcelo Arenas Belón 2019-08-07 5:38 ` René Scharfe 2019-08-07 9:49 ` Carlo Arenas 2019-08-07 13:02 ` René Scharfe 2019-08-07 13:08 ` [PATCH 1/2] nedmalloc: do assignments only after the declaration section René Scharfe 2019-08-07 13:09 ` [PATCH 2/2] nedmalloc: avoid compiler warning about unused value René Scharfe 2019-08-08 2:35 ` [RFC PATCH v3 2/3] grep: make PCRE2 aware of custom allocator Carlo Arenas 2019-08-08 7:07 ` René Scharfe 2019-08-08 12:38 ` Carlo Arenas 2019-08-08 14:29 ` René Scharfe 2019-08-08 20:18 ` Johannes Schindelin 2019-08-07 18:15 ` Junio C Hamano 2019-08-06 16:36 ` [RFC PATCH v3 3/3] grep: avoid leak of chartables in PCRE2 Carlo Marcelo Arenas Belón 2019-08-06 16:48 ` [RFC PATCH v3 0/3] grep: no leaks or crashes (windows testing needed) Junio C Hamano 2019-08-07 21:39 ` [RFC PATCH v4 " Carlo Marcelo Arenas Belón 2019-08-07 21:39 ` [RFC PATCH v4 1/3] grep: make PCRE1 aware of custom allocator Carlo Marcelo Arenas Belón 2019-08-07 21:39 ` [RFC PATCH v4 2/3] grep: make PCRE2 " Carlo Marcelo Arenas Belón 2019-08-07 22:28 ` Junio C Hamano 2019-08-07 21:39 ` [RFC PATCH v4 3/3] grep: avoid leak of chartables in PCRE2 Carlo Marcelo Arenas Belón 2019-08-09 3:02 ` [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes Carlo Marcelo Arenas Belón 2019-08-09 3:02 ` [RFC PATCH v5 1/3] grep: make PCRE1 aware of custom allocator Carlo Marcelo Arenas Belón 2019-08-09 3:02 ` [RFC PATCH v5 2/3] grep: make PCRE2 " Carlo Marcelo Arenas Belón 2019-08-27 9:07 ` Johannes Schindelin 2019-08-27 11:51 ` Carlo Arenas [this message] 2019-10-03 5:02 ` Junio C Hamano 2019-10-03 8:08 ` Johannes Schindelin 2019-10-03 11:17 ` Carlo Arenas 2019-10-03 18:23 ` Johannes Schindelin 2019-10-03 22:57 ` Junio C Hamano 2019-08-09 3:02 ` [RFC PATCH v5 3/3] grep: avoid leak of chartables in PCRE2 Carlo Marcelo Arenas Belón 2019-08-09 11:24 ` [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes Carlo Arenas 2019-08-09 17:01 ` René Scharfe 2019-08-09 17:46 ` Junio C Hamano 2019-08-09 21:26 ` Johannes Schindelin 2019-08-10 3:03 ` [PATCH] SQUASH Carlo Marcelo Arenas Belón 2019-08-10 7:57 ` René Scharfe 2019-08-10 8:42 ` Carlo Arenas 2019-08-10 19:47 ` René Scharfe 2019-08-12 7:35 ` Carlo Arenas 2019-08-12 12:14 ` René Scharfe 2019-08-12 12:28 ` Carlo Arenas 2019-08-10 13:57 ` Johannes Schindelin 2019-08-10 3:05 ` [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes Carlo Arenas 2019-08-10 7:56 ` René Scharfe 2019-08-10 12:40 ` Carlo Arenas 2019-08-10 21:16 ` René Scharfe 2019-08-08 20:21 ` [RFC PATCH v3 0/3] grep: no leaks or crashes (windows testing needed) Johannes Schindelin 2019-08-09 6:52 ` Carlo Arenas 2019-08-09 21:13 ` Johannes Schindelin
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 List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAPUEspjJNSrJQT7xV2fsdp2t5odW5fzzPdDxuar_5x_JPUtf6Q@mail.gmail.com \ --email@example.com \ --cc=Johannes.Schindelin@gmx.de \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator' \ /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
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git 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).