Re: regression in multi-threaded git-pack-index

Thomas Rast [off-list ref] writes:

Thomas Rast [off-list ref] writes:

quoted

  (gdb) r index-pack --keep --stdin -v --pack_header=2,50757 <borked
  Starting program: /Users/trast/.local/bin/git index-pack --keep
--stdin -v --pack_header=2,50757 <borked
  Reading symbols for shared libraries +++........................ done
  Receiving objects: 100% (50757/50757), 24.52 MiB | 13.06 MiB/s, done.
  Resolving deltas:  25% (10568/42272)   
  Program received signal EXC_BAD_ACCESS, Could not access memory.
  Reason: KERN_PROTECTION_FAILURE at address: 0x000000014484dfe8
  [Switching to process 96573 thread 0x10f]
  0x000000010017ee20 in use_pack (p=0x100500f30, w_cursor=0x14484e1a0,
offset=69638148, left=0x0) at sha1_file.c:866
  866             if (!win || !in_window(win, offset)) {

This seems to be a SIGBUS triggered by stack overflow, largely based on
the observation that

  (gdb) p &p
  $6 = (struct packed_git **) 0x144748058

Actually, scratch that; the stack depth is the same no matter what
ulimits I put (up to 64MB).

Actually scratch that again.  It *is* a stack overflow, except that this
is a thread stack, for which the OS X defaults are 512kB apparently, as
opposed to 2MB on linux.

To wit:

  (gdb) p &p
  $11 = (struct packed_git **) 0x14484e058
  (gdb) bt -5
  #4093 0x0000000100054947 in find_unresolved_deltas (base=0x144e00000) at index-pack.c:930
  #4094 0x0000000100054a79 in resolve_base (obj=0x1011b08c0) at index-pack.c:961
  #4095 0x0000000100054ba5 in threaded_second_pass (data=0x100537dd0) at index-pack.c:984
  #4096 0x00007fff8ec8b8bf in _pthread_start ()
  #4097 0x00007fff8ec8eb75 in thread_start ()
  (gdb) f 4094
  #4094 0x0000000100054a79 in resolve_base (obj=0x1011b08c0) at index-pack.c:961
  961             find_unresolved_deltas(base_obj);
  (gdb) p &obj
  $12 = (struct object_entry **) 0x1448cdec8
  (gdb) p 0x14484e058-0x1448cdec8
  $13 = -523888
  (gdb) p 512*1024
  $14 = 524288

And indeed the following patch fixes it.  Sounds like the delta
unpacking needs a rewrite to support stackless operation.  Sigh.

diff --git i/builtin/index-pack.c w/builtin/index-pack.c
index 6be99e2..f73291f 100644
--- i/builtin/index-pack.c
+++ w/builtin/index-pack.c

@@ -1075,13 +1075,17 @@ static void resolve_deltas(void)
 	nr_dispatched = 0;
 	if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) {
 		init_thread();
+		pthread_attr_t attr;
+		pthread_attr_init(&attr);
+		pthread_attr_setstacksize(&attr, 2*1024*1024);
 		for (i = 0; i < nr_threads; i++) {
-			int ret = pthread_create(&thread_data[i].thread, NULL,
+			int ret = pthread_create(&thread_data[i].thread, &attr,
 						 threaded_second_pass, thread_data + i);
 			if (ret)
 				die(_("unable to create thread: %s"),
 				    strerror(ret));
 		}
+		pthread_attr_destroy(&attr);
 		for (i = 0; i < nr_threads; i++)
 			pthread_join(thread_data[i].thread, NULL);
 		cleanup_thread();

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

On Tue, Mar 19, 2013 at 11:11 PM, Thomas Rast [off-list ref] wrote:

Actually scratch that again.  It *is* a stack overflow, except that this
is a thread stack, for which the OS X defaults are 512kB apparently, as
opposed to 2MB on linux.

Thanks. I was scratching my head last night wondering if there was
unprotected access to the object database and probably would have
continued this morning.
-- 
Duy