From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 8D3471F8C2; Sat, 13 Feb 2021 02:19:19 +0000 (UTC) Date: Sat, 13 Feb 2021 02:19:19 +0000 From: Eric Wong To: Xiao Yu Cc: Arkadi Colson , cmogstored-public@yhbt.net Subject: Re: Segfaults on http_close? Message-ID: <20210213021919.GA29494@dcvr> References: <20210117095109.GA28219@dcvr> <20210120085745.GB29704@dcvr> <20210120212229.GA25024@dcvr> <20210125174755.GA30378@dcvr> <20210212065448.GB6193@dcvr> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: List-Id: Xiao Yu wrote: > On Fri, Feb 12, 2021 at 6:54 AM Eric Wong wrote: > > > > Ping? It's been a few weeks :> > > Apologies for not getting back to you earlier! I just checked all of > our logs across our 96 nodes and there's been no segfaults since the > patch was deployed. I think we can call that fix good and tag a new > version. > > Thanks again for the quick fix and diagnosing the issue even with the > limited amount of info I was able to provide. No problem and thanks for the update! I've pushed out the final patch as commit f441da11c290373 and will push out a 1.8.1 release in a few. -------------8<------------ Date: Sat, 13 Feb 2021 01:03:58 +0000 Subject: [PATCH] thrpool: remove stack size changing for all platforms As compilers and system C libraries change, the using a non-default stack size is too risky and can lead to difficult-to-diagnose problems. Using the default stack size seems to solve the segfaults at http_close reported by Xiao Yu . Users on modern 64-bit systems were unlikely to find any benefit in using a small stack size with this code base. Users on 32-bit systems who wish to continue with a minimal stack should use "ulimit -s" in startup scripts or configure their process manager appropriately (e.g. setting the "LimitSTACK" directive in described in systemd.exec(5)). Reported-and-tested-by: Xiao Yu Link: https://yhbt.net/cmogstored-public/CABfxMcW+kb5gwq3pSB_89P49EVv+4UkJXz+mUPQTy19AdrwbAg@mail.gmail.com/T/ --- thrpool.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/thrpool.c b/thrpool.c index bc67ea0..56a7ef0 100644 --- a/thrpool.c +++ b/thrpool.c @@ -12,24 +12,6 @@ struct mog_thr_start_arg { unsigned *do_quit; }; -/* - * we can lower this if we can test with lower values, NPTL minimum is 16K. - * We also use syslog() and *printf() functions which take a lot of - * stack under glibc, so we'll add BUFSIZ (8192 on glibc) to that - */ -#if MOG_LIBKQUEUE /* libkqueue uses quite a bit of stack */ -# define MOG_THR_STACK_SIZE (0) -#elif defined(__GLIBC__) || defined(__FreeBSD__) -# define MOG_THR_STACK_SIZE ((16 * 1024) + MAX(8192,BUFSIZ)) -# if defined(PTHREAD_STACK_MIN) && (PTHREAD_STACK_MIN > MOG_THR_STACK_SIZE) -# undef MOG_THR_STACK_SIZE -# define MOG_THR_STACK_SIZE PTHREAD_STACK_MIN -# endif -#else -# define MOG_THR_STACK_SIZE (0) -#endif -static const size_t stacksize = (size_t)MOG_THR_STACK_SIZE; - static sigset_t quitset; __attribute__((constructor)) static void thrpool_init(void) @@ -141,9 +123,6 @@ thrpool_add(struct mog_thrpool *tp, unsigned size, unsigned long *nr_eagain) CHECK(int, 0, pthread_attr_init(&attr)); - if (stacksize > 0) - CHECK(int, 0, pthread_attr_setstacksize(&attr, stacksize)); - thr = &tp->threads[tp->n_threads].thr; CHECK(int, 0, pthread_mutex_lock(&arg.mtx));