threads / discuss / 54187

possible segfault wint index threading

Subject: possible segfault wint index threading

## tl;dr

3 messages between Sep 4, 2020 and Sep 4, 2020.

replies: 2people: 3as markdown or json

Sandor Bodo-Merle· Sep 4, 2020, 14:08 UTC · lore

Since commit 44c7e1a7e0 (mem-pool: use more standard initialization and finalization, 2020-08-15) i get a SIGSEGV in a linux kernel clone with several remotes added:

(gdb) set args "fetch" "stable" (gdb) run Starting program: /home/sbodo/usr/bin/git "fetch" "stable" [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New Thread 0x7ffff6927700 (LWP 14810)]

Thread 1 "git" received signal SIGSEGV, Segmentation fault.
mem_pool_init (pool=0x0, initial_size=initial_size@entry=7031819) at
mem-pool.c:40
40        memset(pool, 0, sizeof(*pool));
(gdb) bt
#0  mem_pool_init (pool=0x0, initial_size=initial_size@entry=7031819)
at mem-pool.c:40
#1  0x00005555556eb954 in load_cache_entries_threaded
(ieot=0x555555a9f440, nr_threads=<optimized out>, mmap_size=6375563,
mmap=0x7ffff6928000 "DIRC", istate=0x555555a981e0 <the_index>) at
read-cache.c:2109
#2  do_read_index (istate=istate@entry=0x555555a981e0 <the_index>,
path=path@entry=0x555555a9ed50 ".git/index",
must_exist=must_exist@entry=0) at read-cache.c:2229
#3  0x00005555556ef68d in do_read_index (must_exist=0,
path=0x555555a9ed50 ".git/index", istate=0x555555a981e0 <the_index>)
at cache.h:1152
#4  read_index_from (istate=0x555555a981e0 <the_index>,
path=0x555555a9ed50 ".git/index", gitdir=0x555555a9cba0 ".git") at
read-cache.c:2296
#5  0x000055555573e4b0 in get_oid_with_context_1
(repo=repo@entry=0x555555a84dc0 <the_repo>,
name=name@entry=0x5555557e8a92 ":.gitmodules", flags=flags@entry=0,
prefix=prefix@entry=0x0,
    oid=oid@entry=0x7fffffffd890, oc=oc@entry=0x7fffffffd800) at
sha1-name.c:1843
#6  0x000055555573eee3 in get_oid_with_context (oc=0x7fffffffd800,
oid=0x7fffffffd890, flags=0, str=0x5555557e8a92 ":.gitmodules",
repo=0x555555a84dc0 <the_repo>) at sha1-name.c:1940
#7  repo_get_oid (r=r@entry=0x555555a84dc0 <the_repo>,
name=name@entry=0x5555557e8a92 ":.gitmodules",
oid=oid@entry=0x7fffffffd890) at sha1-name.c:1596
#8  0x0000555555748a0f in config_from_gitmodules
(fn=fn@entry=0x555555748b30 <gitmodules_fetch_config>,
repo=0x555555a84dc0 <the_repo>, data=data@entry=0x7fffffffd8f0) at
submodule-config.c:650
#9  0x0000555555749cda in config_from_gitmodules (data=0x7fffffffd8f0,
repo=<optimized out>, fn=0x555555748b30 <gitmodules_fetch_config>) at
submodule-config.c:638
#10 fetch_config_from_gitmodules
(max_children=max_children@entry=0x555555a47994
<submodule_fetch_jobs_config>,
recurse_submodules=recurse_submodules@entry=0x555555a4798c
<recurse_submodules>)
    at submodule-config.c:796
#11 0x00005555555a92bb in cmd_fetch (argc=2, argv=0x7fffffffde20,
prefix=0x0) at builtin/fetch.c:1774
#12 0x000055555557232e in run_builtin (argv=<optimized out>,
argc=<optimized out>, p=<optimized out>) at git.c:450
#13 handle_builtin (argc=<optimized out>, argv=<optimized out>) at git.c:675
#14 0x00005555555734a5 in run_argv (argv=0x7fffffffdb80,
argcp=0x7fffffffdb8c) at git.c:742
#15 cmd_main (argc=<optimized out>, argv=<optimized out>) at git.c:873
#16 0x0000555555571f48 in main (argc=3, argv=0x7fffffffde18) at common-main.c:52

[System Info] git version: git version 2.28.0.394.ge197136389 cpu: x86_64 built from commit: e19713638985533ce461db072b49112da5bd2042 sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Linux 5.4.0-42-generic #46~18.04.1-Ubuntu SMP Fri Jul 10 07:21:24 UTC 2020 x86_64 compiler info: gnuc: 7.5 libc info: glibc: 2.27 $SHELL (typically, interactive shell): /bin/bash

[Enabled Hooks] post-commit post-checkout post-merge post-rewrite

It appears that i have "index.threads" set to "true" in my config. Once i set it to false the segfault goes away.

br
Sandor
René Scharfe· Sep 4, 2020, 17:33 UTC · re: Sandor Bodo-Merle · lore

[PATCH] read-cache: fix mem-pool allocation for multi-threaded index loading

44c7e1a7e0 (mem-pool: use more standard initialization and finalization, 2020-08-15) moved the allocation of the mem-pool structure to callers. It also added an allocation to load_cache_entries_threaded(), but for an unrelated mem-pool. Fix that by allocating the correct one instead -- the one that is initialized two lines later.

Reported-by: Sandor Bodo-Merle <sbodomerle@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
"GIT_TEST_INDEX_THREADS=3 make test" passes with this patch, but fails a
lot without it.
 read-cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index fa291cdbee..ecf6f68994 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2101,7 +2101,7 @@ static unsigned long load_cache_entries_threaded(struct index_state *istate, con
 		nr = 0;
 		for (j = p->ieot_start; j < p->ieot_start + p->ieot_blocks; j++)
 			nr += p->ieot->entries[j].nr;
-		istate->ce_mem_pool = xmalloc(sizeof(*istate->ce_mem_pool));
+		p->ce_mem_pool = xmalloc(sizeof(*istate->ce_mem_pool));
 		if (istate->version == 4) {
 			mem_pool_init(p->ce_mem_pool,
 				estimate_cache_size_from_compressed(nr));
--
2.28.0
Elijah Newren· Sep 4, 2020, 19:28 UTC · re: René Scharfe · lore

Re: [PATCH] read-cache: fix mem-pool allocation for multi-threaded index loading

On Fri, Sep 4, 2020 at 10:34 AM René Scharfe <l.s.r@web.de> wrote:
Show 6 quoted lines
>
> 44c7e1a7e0 (mem-pool: use more standard initialization and finalization,
> 2020-08-15) moved the allocation of the mem-pool structure to callers.
> It also added an allocation to load_cache_entries_threaded(), but for an
> unrelated mem-pool.  Fix that by allocating the correct one instead --
> the one that is initialized two lines later.
Whoops; that's a somewhat embarrassing mistake.  Thanks for fixing it up for me.
Reviewed-by: Elijah Newren <newren@gmail.com>
Show 25 quoted lines
>
> Reported-by: Sandor Bodo-Merle <sbodomerle@gmail.com>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> "GIT_TEST_INDEX_THREADS=3 make test" passes with this patch, but fails a
> lot without it.
>
>  read-cache.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index fa291cdbee..ecf6f68994 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2101,7 +2101,7 @@ static unsigned long load_cache_entries_threaded(struct index_state *istate, con
>                 nr = 0;
>                 for (j = p->ieot_start; j < p->ieot_start + p->ieot_blocks; j++)
>                         nr += p->ieot->entries[j].nr;
> -               istate->ce_mem_pool = xmalloc(sizeof(*istate->ce_mem_pool));
> +               p->ce_mem_pool = xmalloc(sizeof(*istate->ce_mem_pool));
>                 if (istate->version == 4) {
>                         mem_pool_init(p->ce_mem_pool,
>                                 estimate_cache_size_from_compressed(nr));
> --
> 2.28.0

← back to recent threads