git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rich Felker <dalias@aerifal.cx>
To: git@vger.kernel.org
Subject: [PATCH] Ensure sufficient stack space for async threads
Date: Mon, 23 Apr 2012 21:55:23 -0400	[thread overview]
Message-ID: <20120424015523.GA15287@brightrain.aerifal.cx> (raw)

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

The recv_sideband function, which runs in a non-main thread, makes use
of a 64k buffer on the stack. While Linux/glibc systems default to
huge thread stack size (typically 2-10 MB), it's not portable to
assume that the stack for a newly created thread will be able to
support large automatic buffers unless you specifically request space
when creating the thread. The attached patch ensures that at least
128k of stack space is available; certainly that could be increased if
it's deemed safer.

Does this actually matter? Well, the default build of git crashes on
musl libc (www.etalabs.net/musl), where the default stack size is 16k.
We're presently in the process of evaluating what's a good default
stack size for musl to give applications that don't request one
(aiming for a balance that avoids breaking programs like git but also
avoids excess memory usage on tiny embedded systems, a major target
for us) and we'll almost certainly increase the default enough that
the current version of git works without explicitly requesting a stack
size. Nonetheless, this could be an issue again in the future when
running/using git on mobile/embedded-type targets (anybody know
Bionic's default?), and since the patch is simple and has essentially
no cost, I think it's worthwhile to include.

If you have questions please include me in the Cc as I'm not
subscribed.

Rich

[-- Attachment #2: git_pthread_stacksize.diff --]
[-- Type: text/plain, Size: 631 bytes --]

--- run-command.c.orig
+++ run-command.c
@@ -637,11 +637,19 @@
 	async->proc_in = proc_in;
 	async->proc_out = proc_out;
 	{
-		int err = pthread_create(&async->tid, NULL, run_thread, async);
+		int err;
+		pthread_attr_t attr;
+		size_t stacksize;
+		pthread_attr_init(&attr);
+		pthread_attr_getstacksize(&attr, &stacksize);
+		if (stacksize < 131072) stacksize = 131072;
+		pthread_attr_setstacksize(&attr, stacksize);
+		err = pthread_create(&async->tid, &attr, run_thread, async);
 		if (err) {
 			error("cannot create thread: %s", strerror(err));
 			goto error;
 		}
+		pthread_attr_destroy(&attr);
 	}
 #endif
 	return 0;

                 reply	other threads:[~2012-04-24  2:07 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120424015523.GA15287@brightrain.aerifal.cx \
    --to=dalias@aerifal.cx \
    --cc=git@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).