git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator

From
Carlo Arenas <carenas@gmail.com>
Date
Aug 27, 2019, 11:51 UTC
Message-ID
<CAPUEspjJNSrJQT7xV2fsdp2t5odW5fzzPdDxuar_5x_JPUtf6Q@mail.gmail.com>
In-Reply-To
<nycvar.QRO.7.76.6.1908271057280.46@tvgsbejvaqbjf.bet>

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[0] and validated to work thanks to your integration as detailed in [1], 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 [2]

> 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.

Show 10 quoted lines
> 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[3] 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

[0] https://github.com/git/git/commit/0ca5d0550c17a68d83b8922b71aeff891958ed0e [1] https://public-inbox.org/git/CAPUEspiFuvgMQ3W1se1B=8aTTrQsJSZTyQTzG1TpiyN8HTOpkA@mail.gmail.com/ [2] https://public-inbox.org/git/CAPUEspg9F7RutCUCoRAAXmRePjiunq3-zG7cN3uz_t5DVMxP=g@mail.gmail.com/ [3] https://github.com/git/git/pull/627

Previous: Johannes SchindelinNext: Junio C Hamano
Message 43 of 67 in “Fix a problem with PCRE2 and nedmalloc, found via Azure Pipelines”
  1. 0/1 Fix a problem with PCRE2 and nedmalloc, found via Azure PipelinesJohannes Schindelin via GitGitGadget, Aug 5, 2019
  2. 1/1 pcre2: allow overriding the system allocatorJohannes Schindelin via GitGitGadget, Aug 5, 2019
  3. Carlo ArenasAug 5, 2019
  4. Carlo ArenasAug 5, 2019
  5. Johannes SchindelinAug 5, 2019
  6. Johannes SchindelinAug 5, 2019
  7. Junio C HamanoAug 5, 2019
  8. Carlo ArenasAug 6, 2019
  9. 0/3 grep: no leaks (WIP)Carlo Marcelo Arenas Belón, Aug 6, 2019
  10. 1/3 grep: make PCRE1 aware of custom allocatorCarlo Marcelo Arenas Belón, Aug 6, 2019
  11. Johannes SchindelinAug 8, 2019
  12. Carlo ArenasAug 8, 2019
  13. 3/3 grep: avoid leak of chartables in PCRE2Carlo Marcelo Arenas Belón, Aug 6, 2019
  14. 2/3 grep: make PCRE2 aware of custom allocatorCarlo Marcelo Arenas Belón, Aug 6, 2019
  15. Johannes SchindelinAug 8, 2019
  16. Carlo ArenasAug 8, 2019
  17. 0/3 grep: no leaks or crashes (windows testing needed)Carlo Marcelo Arenas Belón, Aug 6, 2019
  18. 2/3 grep: make PCRE2 aware of custom allocatorCarlo Marcelo Arenas Belón, Aug 6, 2019
  19. René ScharfeAug 7, 2019
  20. Carlo ArenasAug 7, 2019
  21. René ScharfeAug 7, 2019
  22. 1/2 nedmalloc: do assignments only after the declaration sectionRené Scharfe, Aug 7, 2019
  23. 2/2 nedmalloc: avoid compiler warning about unused valueRené Scharfe, Aug 7, 2019
  24. Carlo ArenasAug 8, 2019
  25. René ScharfeAug 8, 2019
  26. Carlo ArenasAug 8, 2019
  27. René ScharfeAug 8, 2019
  28. Johannes SchindelinAug 8, 2019
  29. Junio C HamanoAug 7, 2019
  30. 1/3 grep: make PCRE1 aware of custom allocatorCarlo Marcelo Arenas Belón, Aug 6, 2019
  31. 3/3 grep: avoid leak of chartables in PCRE2Carlo Marcelo Arenas Belón, Aug 6, 2019
  32. Junio C HamanoAug 6, 2019
  33. 0/3 grep: no leaks or crashes (windows testing needed)Carlo Marcelo Arenas Belón, Aug 7, 2019
  34. 1/3 grep: make PCRE1 aware of custom allocatorCarlo Marcelo Arenas Belón, Aug 7, 2019
  35. 2/3 grep: make PCRE2 aware of custom allocatorCarlo Marcelo Arenas Belón, Aug 7, 2019
  36. Junio C HamanoAug 7, 2019
  37. 3/3 grep: avoid leak of chartables in PCRE2Carlo Marcelo Arenas Belón, Aug 7, 2019
  38. 0/3 grep: almost no more leaks, hopefully no crashesCarlo Marcelo Arenas Belón, Aug 9, 2019
  39. 1/3 grep: make PCRE1 aware of custom allocatorCarlo Marcelo Arenas Belón, Aug 9, 2019
  40. 3/3 grep: avoid leak of chartables in PCRE2Carlo Marcelo Arenas Belón, Aug 9, 2019
  41. 2/3 grep: make PCRE2 aware of custom allocatorCarlo Marcelo Arenas Belón, Aug 9, 2019
  42. Johannes SchindelinAug 27, 2019
  43. Carlo ArenasAug 27, 2019
  44. Junio C HamanoOct 3, 2019
  45. Johannes SchindelinOct 3, 2019
  46. Carlo ArenasOct 3, 2019
  47. Johannes SchindelinOct 3, 2019
  48. Junio C HamanoOct 3, 2019
  49. Carlo ArenasAug 9, 2019
  50. René ScharfeAug 9, 2019
  51. Junio C HamanoAug 9, 2019
  52. Johannes SchindelinAug 9, 2019
  53. SQUASHCarlo Marcelo Arenas Belón, Aug 10, 2019
  54. René ScharfeAug 10, 2019
  55. Carlo ArenasAug 10, 2019
  56. René ScharfeAug 10, 2019
  57. Carlo ArenasAug 12, 2019
  58. René ScharfeAug 12, 2019
  59. Carlo ArenasAug 12, 2019
  60. Johannes SchindelinAug 10, 2019
  61. Carlo ArenasAug 10, 2019
  62. René ScharfeAug 10, 2019
  63. Carlo ArenasAug 10, 2019
  64. René ScharfeAug 10, 2019
  65. Johannes SchindelinAug 8, 2019
  66. Carlo ArenasAug 9, 2019
  67. Johannes SchindelinAug 9, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.