Re: ps/reftable-sans-compat-util, was Re: What's cooking in git.git (Feb 2025, #09; Fri, 28)

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 4, 2025, 10:06 UTC
Message-ID: <Z8bQsflIqzZd3AMs@pks.im>
In-Reply-To: <Z8bL0HUuQqj7LlTp@pks.im>

On Tue, Mar 04, 2025 at 10:46:17AM +0100, Patrick Steinhardt wrote:

Show 35 quoted lines

> On Tue, Mar 04, 2025 at 08:40:13AM +0100, Johannes Schindelin wrote:
> > On Tue, 4 Mar 2025, Johannes Schindelin wrote:
> > > On Tue, 4 Mar 2025, Patrick Steinhardt wrote:
> > > > Johannes, did the new version of this patch series make your life any
> > > > easier? As far as I can see the conflicts in your "shears/seen" branch
> > > > seem to have been fixed, and the failing pipeline seems to be due to
> > > > other issues. If so, we would be able to move ahead with this patch
> > > > series and the dependent fix for Windows below.
> > >
> > > Honestly, I cannot say whether it made my life any easier. As you can see
> > > from
> > > https://github.com/git-for-windows/git/actions/workflows/main.yml?query=branch%3Ashears%2Fseen,
> > > the `shears/seen` branch failed to update automatically since I updated it
> > > manually last Thursday. According to
> > > https://github.com/git/git/activity?ref=seen, there have been 5 updates
> > > that were hence missed.
> > >
> > > I'll try to update the `shears/seen` branch now, but I'll time-box it to
> > > half an hour because I really planned on taking care of other issues.
> > 
> > It seems that there are the usual CMake-related breakages (not related to
> > ps/reftable-sans-compat-util, but to the introduction of the
> > `CLAR_TEST_OBJS` or at least the `lib-oid` addition to that). I hope that
> > the tip commit of `shears/seen` that I just pushed addresses that, but I
> > ran out of time to monitor this.
> > 
> > And there are some pretty bad `exit code 127` problems in the unit tests
> > on Windows, see e.g.
> > https://github.com/git-for-windows/git/actions/runs/13648196783/job/38151312208#step:5:213
> > (but again, I ran out of the allotted time).
> 
> Interesting. All of the errors relate to reftable stuff. Curiously,
> those errors only seem to happen in the MinGW build, but not with the
> Meson-based MSVC build. I can reproduce the issue in MinGW indeed, so
> I'll investigate. Thanks for the hint!

Okay, I found the issue: it's mismatching allocators. Git for Windows has support for the custom mimalloc allocator, and it's getting roped in by default in MinGW builds. And because the allocator is declared in "git-compat-util.h", the reftable library uses allocators from mscrt.dll, whereas the rest of the Git code base uses allocators from mimalloc. This causes us to sometimes free memory with a different allocator than we have allocated it with, and that causes a SIGTRAP.

Below patch addresses the issue.

Patrick

diff --git a/compat/posix.h b/compat/posix.h
index b484029f751..5ad3539d55b 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -176,6 +176,16 @@ typedef unsigned long uintptr_t;
 #define _ALL_SOURCE 1
 #endif
 
+#ifdef USE_MIMALLOC
+#include "mimalloc.h"
+#define malloc mi_malloc
+#define calloc mi_calloc
+#define realloc mi_realloc
+#define free mi_free
+#define strdup mi_strdup
+#define strndup mi_strndup
+#endif
+
 #ifdef MKDIR_WO_TRAILING_SLASH
 #define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b))
 int compat_mkdir_wo_trailing_slash(const char*, mode_t);
diff --git a/git-compat-util.h b/git-compat-util.h
index 8d2acf86670..a102a365592 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -226,16 +226,6 @@ static inline const char *precompose_string_if_needed(const char *in)
 # include <sys/sysinfo.h>
 #endif
 
-#ifdef USE_MIMALLOC
-#include "mimalloc.h"
-#define malloc mi_malloc
-#define calloc mi_calloc
-#define realloc mi_realloc
-#define free mi_free
-#define strdup mi_strdup
-#define strndup mi_strndup
-#endif
-
 #ifndef PATH_SEP
 #define PATH_SEP ':'
 #endif