~alpine/aports

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
8 2

[alpine-aports] [PATCH] testing/mongodb: fix bug #5117 where mongo fails at start

Details
Message ID
<1456083383-9196-1-git-send-email-marc.vertes@ugrid.net>
Sender timestamp
1456083383
DKIM signature
missing
Download raw message
Patch: +29 -1
The check for recursion has been disabled, as it makes mongo shell
aborting at start. There may be a better fix, perhaps ensuring
that recursion is OK under musl, and maybe increasing stack space ?
---
 testing/mongodb/APKBUILD                  |  7 ++++++-
 testing/mongodb/mongo-fix-recursion.patch | 23 +++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 testing/mongodb/mongo-fix-recursion.patch

diff --git a/testing/mongodb/APKBUILD b/testing/mongodb/APKBUILD
index 9911519..14476cf 100644
--- a/testing/mongodb/APKBUILD
+++ b/testing/mongodb/APKBUILD
@@ -19,6 +19,7 @@ source="http://downloads.mongodb.org/src/mongodb-src-r${pkgver}.tar.gz
	backtrace.patch
	fix-asio-strerror_r.patch
	boost160.patch
	mongo-fix-recursion.patch

        mongodb.confd
        mongodb.initd
@@ -31,7 +32,8 @@ source="http://downloads.mongodb.org/src/mongodb-src-r${pkgver}.tar.gz
# 2. Use system allocator as tc-malloc doesn't build
# 2. Use as much system libs as possible, boost doesn't compile foe example
#
# TODO: proper elf-netive-class fix
# TODO: proper fix for recursion check and/or stack management in mongo shell
# TODO: proper elf-native-class fix
# TODO: check if there are more libs could be replaced by system counterparts (see libpcap does't use, for example)
# TODO: proper fix for heap usage
# Right now code patched to always return 0 for heap usage statistics. That is necessary because musl malloc info
@@ -96,6 +98,7 @@ md5sums="a9f150f8bead9e8afd3ab28492167073  mongodb-src-r3.2.1.tar.gz
86a988b5d4402227d177b8a1167008e8  backtrace.patch
cd0833592e3a23e729ebd71eb756318c  fix-asio-strerror_r.patch
1df24dc2aa6b8f4c6da22f097a921ebb  boost160.patch
8161b9a08aef17dd171fc4979b50315f  mongo-fix-recursion.patch
7d2f94bed7bfacd32fcd52dfd931f077  mongodb.confd
792a0b53b3e843cf14176c5beb8cdfe1  mongodb.initd
49df78833de4cb6e2b9b1ab9da52c3ac  mongodb.logrotate
@@ -107,6 +110,7 @@ sha256sums="50431a3ba5ab68bd0bed4a157a8528ca27753a63cf101f13135255e4e9d42f15  mo
300d9f6b819730de54018d4b418eb7f921ba9c83fad4958a040544b076160848  backtrace.patch
ec6d404221f02706ef2e52e00e414e98666dcc3606e78c9b3d33dfbd42a1eae7  fix-asio-strerror_r.patch
0e9da35f4303e53daf51e78961c517895f2d12f4fa49298f01e1665e15246d73  boost160.patch
d9822952baf5a265e1440ee284e0890aa83cfd7b52438fadb4f311f1d440ef27  mongo-fix-recursion.patch
a4ca29c577428c02cd0b0a8b46756df5f53a05519c9d13c270533cf99b9b819d  mongodb.confd
7e39fbd4dc18dba21c8767931683f4795e58e0e91b9f9a5842539923ded453c9  mongodb.initd
76994c32d999def5c925bd7be3f96687b3406f1d67b89aa6a4df8053025b1e01  mongodb.logrotate
@@ -118,6 +122,7 @@ b9fdacb273d5a4e1e60735846b262287f84ca6cbded9393d182f69158d3162a50cae5d834f76860d
7d097f497cb910c9cb81086cd8c222e43456d1a6de4adfe3e97a4d99add454279350fdeb7305dab84b3deca04afd824036d4065ee0fb8cdd8c03e96d98ee86af  backtrace.patch
f829b1ad542db3ee776d444243b8b47ab4e48a7386d9b199d7b1adafd31556cf173a5683b78ee735d6a69999ad9af5ad152fde955bbe8865f7910718991ce97c  fix-asio-strerror_r.patch
385c82875174caae433a3b381eb10f98a6fed0c8943788ddefff1de80a898e480bdbbf094a7783285cf2ae11ce3fc6878e57d58183d05be2f0837b206aaa4da6  boost160.patch
209a062524a447f54ccf2da9f22fc2946d08b6e119eb1f91d4b4420fdc1de15fd5d91974e284edcecd1b4a4b3bea82bd80e361125a57d9ade0b4503c920d3d9f  mongo-fix-recursion.patch
9bcd870742c31bf25f34188ddc3c414de1103e9860dea9f54eee276b89bc2cf1226abab1749c5cda6a6fb0880e541373754e5e83d63cc7189d4b9c274fd555c3  mongodb.confd
2cb295ac0eb44acb4533c86d6d0988d5f760361a43cff7845713f3e2cf0e37023d23790a2926c613bf2b0668060c0b68d59000db52daaacd8af10e701a8b4192  mongodb.initd
8c089b1a11f494e4148fb4646265964c925bf937633a65e395ee1361d42facf837871dd493a9a2e0f480ae0e0829dbd3ed60794c5334e2716332e131fc5c2c51  mongodb.logrotate
diff --git a/testing/mongodb/mongo-fix-recursion.patch b/testing/mongodb/mongo-fix-recursion.patch
new file mode 100644
index 0000000..ba147a2
--- /dev/null
+++ b/testing/mongodb/mongo-fix-recursion.patch
@@ -0,0 +1,23 @@
--- a/src/third_party/mozjs-38/extract/js/src/jsfriendapi.h
+++ b/src/third_party/mozjs-38/extract/js/src/jsfriendapi.h
@@ -972,6 +972,12 @@
  * a little more from it).
  */
 
+// Under alpinelinux (musl libc), mongo shell fails immediately with 
+// "too much recursion".
+// fix the real cause instead of disabling the check.
+#if 1	
+#define JS_CHECK_RECURSION_LIMIT(cx, limit, onerror)
+#else
 #define JS_CHECK_RECURSION_LIMIT(cx, limit, onerror)                            \
     JS_BEGIN_MACRO                                                              \
         int stackDummy_;                                                        \
@@ -980,6 +986,7 @@
             onerror;                                                            \
         }                                                                       \
     JS_END_MACRO
+#endif
 
 #define JS_CHECK_RECURSION(cx, onerror)                                         \
     JS_CHECK_RECURSION_LIMIT(cx, js::GetNativeStackLimit(cx), onerror)
-- 
2.7.1



---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Timo Teras <timo.teras@iki.fi>
Details
Message ID
<20160222085745.4a1ac344@vostro.util.wtbts.net>
In-Reply-To
<1456083383-9196-1-git-send-email-marc.vertes@ugrid.net> (view parent)
Sender timestamp
1456124265
DKIM signature
missing
Download raw message
On Sun, 21 Feb 2016 19:36:23 +0000
Marc Vertes <marc.vertes@ugrid.net> wrote:

> The check for recursion has been disabled, as it makes mongo shell
> aborting at start. There may be a better fix, perhaps ensuring
> that recursion is OK under musl, and maybe increasing stack space ?

The problem is like the fact that the app uses pthread_getattr_np to
get the main thread's attributes, and then pthread_attr_getstack to
figure out the main stack's size.

Musl in this case seems to report the actual committed stack size.
Basically current stack size rounded up to one or two pages. Where as
the application is likely expecting to get the upper bound to which the
stack can grow - basically the virtual address space size reserved by
the kernel for stack.

As alternative it was suggested to use getrlimit(RLIMIT_STACK) for the
main thread.

I guess this is subject to debate which is the correct thing to do in
pthread_attr_getstack for the main thread. Returning the commit size
gives you size that you can access and it's guaranteed to not fail.

We are currently discussing this on #musl. But perhaps it'd be better
to do it on the mailing list.

Thank,s
Timo


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Timo Teras <timo.teras@iki.fi>
Details
Message ID
<20160223150743.33e55efa@vostro.util.wtbts.net>
In-Reply-To
<1456083383-9196-1-git-send-email-marc.vertes@ugrid.net> (view parent)
Sender timestamp
1456232863
DKIM signature
missing
Download raw message
Patch: +27 -1
On Sun, 21 Feb 2016 19:36:23 +0000
Marc Vertes <marc.vertes@ugrid.net> wrote:

> The check for recursion has been disabled, as it makes mongo shell
> aborting at start. There may be a better fix, perhaps ensuring
> that recursion is OK under musl, and maybe increasing stack space ?

Could you try if the following works instead:

From e9cbe2a20b5d4b7a951ba41236feb0523241c3ea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timo=20Ter=C3=A4s?= <timo.teras@iki.fi>
Date: Tue, 23 Feb 2016 13:01:26 +0000
Subject: [PATCH] testing/mongodb: use getrlimit() to get main process stack
 size

---
 testing/mongodb/APKBUILD                      |  6 +++++-
 testing/mongodb/musl-process-stack-size.patch | 22 ++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 testing/mongodb/musl-process-stack-size.patch

diff --git a/testing/mongodb/APKBUILD b/testing/mongodb/APKBUILD
index 9911519..a177e91 100644
--- a/testing/mongodb/APKBUILD
+++ b/testing/mongodb/APKBUILD
@@ -2,7 +2,7 @@

pkgname=mongodb
pkgver=3.2.1
pkgrel=1
pkgrel=2
pkgdesc='A high-performance, open source, schema-free document-oriented database'
url='http://www.mongodb.org'
arch='x86_64'
@@ -18,6 +18,7 @@ source="http://downloads.mongodb.org/src/mongodb-src-r${pkgver}.tar.gz
	40-fix-elf-native-class.patch
	backtrace.patch
	fix-asio-strerror_r.patch
	musl-process-stack-size.patch
	boost160.patch

        mongodb.confd
@@ -95,6 +96,7 @@ md5sums="a9f150f8bead9e8afd3ab28492167073  mongodb-src-r3.2.1.tar.gz
04a348397be8ca7471d404056d8a1490  40-fix-elf-native-class.patch
86a988b5d4402227d177b8a1167008e8  backtrace.patch
cd0833592e3a23e729ebd71eb756318c  fix-asio-strerror_r.patch
4a0b1b35ee0c6a5e5c35aee0097f4a07  musl-process-stack-size.patch
1df24dc2aa6b8f4c6da22f097a921ebb  boost160.patch
7d2f94bed7bfacd32fcd52dfd931f077  mongodb.confd
792a0b53b3e843cf14176c5beb8cdfe1  mongodb.initd
@@ -106,6 +108,7 @@ sha256sums="50431a3ba5ab68bd0bed4a157a8528ca27753a63cf101f13135255e4e9d42f15  mo
3a863660113d29728d7a852b3fba73926697b496848f8ccc4d8e73e6614cfdfc  40-fix-elf-native-class.patch
300d9f6b819730de54018d4b418eb7f921ba9c83fad4958a040544b076160848  backtrace.patch
ec6d404221f02706ef2e52e00e414e98666dcc3606e78c9b3d33dfbd42a1eae7  fix-asio-strerror_r.patch
a266b1b2397f069fedb2427a1989a30ed9f63a300ba8726ec50609e93920f76d  musl-process-stack-size.patch
0e9da35f4303e53daf51e78961c517895f2d12f4fa49298f01e1665e15246d73  boost160.patch
a4ca29c577428c02cd0b0a8b46756df5f53a05519c9d13c270533cf99b9b819d  mongodb.confd
7e39fbd4dc18dba21c8767931683f4795e58e0e91b9f9a5842539923ded453c9  mongodb.initd
@@ -117,6 +120,7 @@ b9fdacb273d5a4e1e60735846b262287f84ca6cbded9393d182f69158d3162a50cae5d834f76860d
56db8f43afc94713ac65b174189e2dd903b5e1eff0b65f1bdac159e52ad4de6606c449865d73bd47bffad6a8fc339777e2b11395596e9a476867d94a219c7925  40-fix-elf-native-class.patch
7d097f497cb910c9cb81086cd8c222e43456d1a6de4adfe3e97a4d99add454279350fdeb7305dab84b3deca04afd824036d4065ee0fb8cdd8c03e96d98ee86af  backtrace.patch
f829b1ad542db3ee776d444243b8b47ab4e48a7386d9b199d7b1adafd31556cf173a5683b78ee735d6a69999ad9af5ad152fde955bbe8865f7910718991ce97c  fix-asio-strerror_r.patch
e8f41ea4df62b6588cdea64e1f57553e04c3c0b603dac68b49b222bc9fa8c7b6f6bc1654b5033318ad3ec73632848dfa0f03d33b3c8d8b523ef0145e461364b2  musl-process-stack-size.patch
385c82875174caae433a3b381eb10f98a6fed0c8943788ddefff1de80a898e480bdbbf094a7783285cf2ae11ce3fc6878e57d58183d05be2f0837b206aaa4da6  boost160.patch
9bcd870742c31bf25f34188ddc3c414de1103e9860dea9f54eee276b89bc2cf1226abab1749c5cda6a6fb0880e541373754e5e83d63cc7189d4b9c274fd555c3  mongodb.confd
2cb295ac0eb44acb4533c86d6d0988d5f760361a43cff7845713f3e2cf0e37023d23790a2926c613bf2b0668060c0b68d59000db52daaacd8af10e701a8b4192  mongodb.initd
diff --git a/testing/mongodb/musl-process-stack-size.patch b/testing/mongodb/musl-process-stack-size.patch
new file mode 100644
index 0000000..dd45abb
--- /dev/null
+++ b/testing/mongodb/musl-process-stack-size.patch
@@ -0,0 +1,22 @@
--- mongodb-src-r3.2.1.orig/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
+++ mongodb-src-r3.2.1/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
@@ -31,6 +31,7 @@
 #include "mongo/platform/stack_locator.h"
 
 #include <pthread.h>
+#include <sys/resource.h>
 
 #include "mongo/util/assert_util.h"
 #include "mongo/util/scopeguard.h"
@@ -52,6 +53,11 @@
     invariant(result == 0);
     invariant(base != nullptr);
     invariant(size != 0);
+
+    struct rlimit rl;
+
+    invariant(getrlimit(RLIMIT_STACK, &rl) == 0);
+    size = rl.rlim_cur ? : 2 * 1024 * 1024;
 
     // TODO: Assumes a downward growing stack. Note here that
     // getstack returns the stack *base*, being the bottom of the
-- 
2.7.1




---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Details
Message ID
<9D775D86-8A4D-49ED-8512-EBD572ECF6D6@ugrid.net>
In-Reply-To
<20160223150743.33e55efa@vostro.util.wtbts.net> (view parent)
Sender timestamp
1456238483
DKIM signature
missing
Download raw message
> Le 23 févr. 2016 à 14:07, Timo Teras <timo.teras@iki.fi> a écrit :
> 
> On Sun, 21 Feb 2016 19:36:23 +0000
> Marc Vertes <marc.vertes@ugrid.net> wrote:
> 
>> The check for recursion has been disabled, as it makes mongo shell
>> aborting at start. There may be a better fix, perhaps ensuring
>> that recursion is OK under musl, and maybe increasing stack space ?
> 
> Could you try if the following works instead:
> 
> 
> --- /dev/null
> +++ b/testing/mongodb/musl-process-stack-size.patch
> @@ -0,0 +1,22 @@
> +--- mongodb-src-r3.2.1.orig/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
> ++++ mongodb-src-r3.2.1/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
> +@@ -31,6 +31,7 @@
> + #include "mongo/platform/stack_locator.h"
> + 
> + #include <pthread.h>
> ++#include <sys/resource.h>
> + 
> + #include "mongo/util/assert_util.h"
> + #include "mongo/util/scopeguard.h"
> +@@ -52,6 +53,11 @@
> +     invariant(result == 0);
> +     invariant(base != nullptr);
> +     invariant(size != 0);
> ++
> ++    struct rlimit rl;
> ++
> ++    invariant(getrlimit(RLIMIT_STACK, &rl) == 0);
> ++    size = rl.rlim_cur ? : 2 * 1024 * 1024;
> + 
> +     // TODO: Assumes a downward growing stack. Note here that
> +     // getstack returns the stack *base*, being the bottom of the
> -- 
> 2.7.1
> 
It doesn't work. And on my side, I tried a similar modification (using getrlimit) in src/third_party/mozjs-38/extract/js/src/jsnativestack.cpp without success. 

But I found a separate patch which works:

--- src/mongo/scripting/mozjs/implscope.cpp.orig
+++ src/mongo/scripting/mozjs/implscope.cpp
@@ -258,6 +258,7 @@
             //
             // TODO: What if we are running on a platform with very
             // large pages, like 4MB?
+            //JS_SetNativeStackQuota(_runtime, available.get() - (64 * 1024));
             JS_SetNativeStackQuota(_runtime, available.get() - (32 * 1024));
         }
 
So I believe that it should be a matter of increasing allocated stack at thread creation. I'm still not able to do that, despite my various attempts.

Marc

---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Details
Message ID
<E7A49593-05F0-4188-921C-7A4B1868D4FC@ugrid.net>
In-Reply-To
<20160223170446.483be803@vostro.util.wtbts.net> (view parent)
Sender timestamp
1456241758
DKIM signature
missing
Download raw message
> Le 23 févr. 2016 à 16:04, Timo Teras <timo.teras@iki.fi> a écrit :
> 
> On Tue, 23 Feb 2016 15:41:23 +0100
> Marc Vertes <marc.vertes@ugrid.net> wrote:
> 
>> It doesn't work. And on my side, I tried a similar modification
>> (using getrlimit) in
>> src/third_party/mozjs-38/extract/js/src/jsnativestack.cpp without
>> success. 
>> 
>> But I found a separate patch which works:
>> 
>> --- src/mongo/scripting/mozjs/implscope.cpp.orig
>> +++ src/mongo/scripting/mozjs/implscope.cpp
>> @@ -258,6 +258,7 @@
>>             //
>>             // TODO: What if we are running on a platform with very
>>             // large pages, like 4MB?
>> +            //JS_SetNativeStackQuota(_runtime, available.get() - (64
>> * 1024)); JS_SetNativeStackQuota(_runtime, available.get() - (32 *
>> 1024)); }
>> 
>> So I believe that it should be a matter of increasing allocated stack
>> at thread creation. I'm still not able to do that, despite my various
>> attempts.
> 
> The main thread's stack is allocated by kernel. In practice, kernel
> maps the stack on virtual address space so that it's size is the
> stack rlimit, but it only allocates the first pages of it. When the
> stack is needed more, the kernel automatically commits/allocates more
> of the stack.
> 
> Musl returns you the actual size of the stack in use. Glibc returns you
> the maximum stack size (but kernel might fail to allocate it).
> 
> Another attempt would be to patch that function to have in the
> beginning a call to function that has large stack array and that writes
> to it.
> 
> Something like:
> 
> static void commit_stack(void)
> {
>  char foo[1024*1024*2];
>  foo[0] = foo[sizeof(foo)-1] = 0;
> }
> 
> Assuming gcc will not optimize it away.
> 
> After this kernel should commit the memory, and musl's
> pthread_getattr_np should return value reflecting that.

No success. I think that the base address returned by pthread_attr_getstack(&attr, &base, &size)
is the problem. Increasing the size (like with getrlimit) doesn't change the test in StackLocator.available() which triggers abort:
it checks the available space against this base address (&localaddr - base > quota).

Changing size does nothing.

I also noticed that pthread_getattr_np and pthread_attr_getstack where not performed from the main thread (gettid != getpid)

Marc

---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Timo Teras <timo.teras@iki.fi>
Details
Message ID
<20160223170446.483be803@vostro.util.wtbts.net>
In-Reply-To
<9D775D86-8A4D-49ED-8512-EBD572ECF6D6@ugrid.net> (view parent)
Sender timestamp
1456239886
DKIM signature
missing
Download raw message
On Tue, 23 Feb 2016 15:41:23 +0100
Marc Vertes <marc.vertes@ugrid.net> wrote:

> It doesn't work. And on my side, I tried a similar modification
> (using getrlimit) in
> src/third_party/mozjs-38/extract/js/src/jsnativestack.cpp without
> success. 
> 
> But I found a separate patch which works:
> 
> --- src/mongo/scripting/mozjs/implscope.cpp.orig
> +++ src/mongo/scripting/mozjs/implscope.cpp
> @@ -258,6 +258,7 @@
>              //
>              // TODO: What if we are running on a platform with very
>              // large pages, like 4MB?
> +            //JS_SetNativeStackQuota(_runtime, available.get() - (64
> * 1024)); JS_SetNativeStackQuota(_runtime, available.get() - (32 *
> 1024)); }
>  
> So I believe that it should be a matter of increasing allocated stack
> at thread creation. I'm still not able to do that, despite my various
> attempts.

The main thread's stack is allocated by kernel. In practice, kernel
maps the stack on virtual address space so that it's size is the
stack rlimit, but it only allocates the first pages of it. When the
stack is needed more, the kernel automatically commits/allocates more
of the stack.

Musl returns you the actual size of the stack in use. Glibc returns you
the maximum stack size (but kernel might fail to allocate it).

Another attempt would be to patch that function to have in the
beginning a call to function that has large stack array and that writes
to it.

Something like:

static void commit_stack(void)
{
  char foo[1024*1024*2];
  foo[0] = foo[sizeof(foo)-1] = 0;
}

Assuming gcc will not optimize it away.

After this kernel should commit the memory, and musl's
pthread_getattr_np should return value reflecting that.

/Timo


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Details
Message ID
<339A4F58-917F-455E-9B0E-E1E5A5902ADE@ugrid.net>
In-Reply-To
<20160223175029.23bcadb2@vostro> (view parent)
Sender timestamp
1456243844
DKIM signature
missing
Download raw message
> Le 23 févr. 2016 à 16:50, Timo Teras <timo.teras@iki.fi> a écrit :
> 
> On Tue, 23 Feb 2016 16:35:58 +0100
> Marc Vertes <marc.vertes@ugrid.net> wrote:
> 
>> No success. I think that the base address returned by
>> pthread_attr_getstack(&attr, &base, &size) is the problem. Increasing
>> the size (like with getrlimit) doesn't change the test in
>> StackLocator.available() which triggers abort: it checks the
>> available space against this base address (&localaddr - base > quota).
>> 
>> Changing size does nothing.
>> 
>> I also noticed that pthread_getattr_np and pthread_attr_getstack
>> where not performed from the main thread (gettid != getpid)
> 
> Right. ulimit should be used only for main thread. Not the created
> threads with libc allocated stack. That needs a fix.
> 
> pthread_attr_getstack should return the lowest addressable byte of the
> stack. So indeed, if we change the size, we need to adjust base
> accordingly on system with downward growing stack. Something like:
> 
> --- mongodb-src-r3.2.1.orig/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
> +++ mongodb-src-r3.2.1/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
> @@ -31,6 +31,7 @@
> #include "mongo/platform/stack_locator.h"
> 
> #include <pthread.h>
> +#include <sys/resource.h>
> 
> #include "mongo/util/assert_util.h"
> #include "mongo/util/scopeguard.h"
> @@ -52,6 +53,13 @@
>     invariant(result == 0);
>     invariant(base != nullptr);
>     invariant(size != 0);
> +
> +    struct rlimit rl;
> +
> +    base += size;
> +    invariant(getrlimit(RLIMIT_STACK, &rl) == 0);
> +    size = rl.rlim_cur ? : 2 * 1024 * 1024;
> +    base -= size;
> 
>     // TODO: Assumes a downward growing stack. Note here that
>     // getstack returns the stack *base*, being the bottom of the
> 

This one works!
But I'm afraid of possible stack overlap between threads if it is run not from main thread. Is it your concern too ?

Marc

---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Timo Teras <timo.teras@iki.fi>
Details
Message ID
<20160223175029.23bcadb2@vostro>
In-Reply-To
<E7A49593-05F0-4188-921C-7A4B1868D4FC@ugrid.net> (view parent)
Sender timestamp
1456242628
DKIM signature
missing
Download raw message
On Tue, 23 Feb 2016 16:35:58 +0100
Marc Vertes <marc.vertes@ugrid.net> wrote:

> No success. I think that the base address returned by
> pthread_attr_getstack(&attr, &base, &size) is the problem. Increasing
> the size (like with getrlimit) doesn't change the test in
> StackLocator.available() which triggers abort: it checks the
> available space against this base address (&localaddr - base > quota).
> 
> Changing size does nothing.
> 
> I also noticed that pthread_getattr_np and pthread_attr_getstack
> where not performed from the main thread (gettid != getpid)

Right. ulimit should be used only for main thread. Not the created
threads with libc allocated stack. That needs a fix.

pthread_attr_getstack should return the lowest addressable byte of the
stack. So indeed, if we change the size, we need to adjust base
accordingly on system with downward growing stack. Something like:

--- mongodb-src-r3.2.1.orig/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
+++ mongodb-src-r3.2.1/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
@@ -31,6 +31,7 @@
 #include "mongo/platform/stack_locator.h"
 
 #include <pthread.h>
+#include <sys/resource.h>
 
 #include "mongo/util/assert_util.h"
 #include "mongo/util/scopeguard.h"
@@ -52,6 +53,13 @@
     invariant(result == 0);
     invariant(base != nullptr);
     invariant(size != 0);
+
+    struct rlimit rl;
+
+    base += size;
+    invariant(getrlimit(RLIMIT_STACK, &rl) == 0);
+    size = rl.rlim_cur ? : 2 * 1024 * 1024;
+    base -= size;
 
     // TODO: Assumes a downward growing stack. Note here that
     // getstack returns the stack *base*, being the bottom of the



---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Timo Teras <timo.teras@iki.fi>
Details
Message ID
<20160223182010.039b4469@vostro>
In-Reply-To
<339A4F58-917F-455E-9B0E-E1E5A5902ADE@ugrid.net> (view parent)
Sender timestamp
1456244410
DKIM signature
missing
Download raw message
On Tue, 23 Feb 2016 17:10:44 +0100
Marc Vertes <marc.vertes@ugrid.net> wrote:

> > Le 23 févr. 2016 à 16:50, Timo Teras <timo.teras@iki.fi> a écrit :
> > 
> > On Tue, 23 Feb 2016 16:35:58 +0100
> > Marc Vertes <marc.vertes@ugrid.net> wrote:
> >   
> >> No success. I think that the base address returned by
> >> pthread_attr_getstack(&attr, &base, &size) is the problem.
> >> Increasing the size (like with getrlimit) doesn't change the test
> >> in StackLocator.available() which triggers abort: it checks the
> >> available space against this base address (&localaddr - base >
> >> quota).
> >> 
> >> Changing size does nothing.
> >> 
> >> I also noticed that pthread_getattr_np and pthread_attr_getstack
> >> where not performed from the main thread (gettid != getpid)  
> > 
> > Right. ulimit should be used only for main thread. Not the created
> > threads with libc allocated stack. That needs a fix.
> > 
> > pthread_attr_getstack should return the lowest addressable byte of
> > the stack. So indeed, if we change the size, we need to adjust base
> > accordingly on system with downward growing stack. Something like:
> > 
> > ---
> > mongodb-src-r3.2.1.orig/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
> > +++
> > mongodb-src-r3.2.1/src/mongo/platform/stack_locator_pthread_getattr_np.cpp
> > @@ -31,6 +31,7 @@ #include "mongo/platform/stack_locator.h"
> > 
> > #include <pthread.h>
> > +#include <sys/resource.h>
> > 
> > #include "mongo/util/assert_util.h"
> > #include "mongo/util/scopeguard.h"
> > @@ -52,6 +53,13 @@
> >     invariant(result == 0);
> >     invariant(base != nullptr);
> >     invariant(size != 0);
> > +
> > +    struct rlimit rl;
> > +
> > +    base += size;
> > +    invariant(getrlimit(RLIMIT_STACK, &rl) == 0);
> > +    size = rl.rlim_cur ? : 2 * 1024 * 1024;
> > +    base -= size;
> > 
> >     // TODO: Assumes a downward growing stack. Note here that
> >     // getstack returns the stack *base*, being the bottom of the
> >   
> 
> This one works!
> But I'm afraid of possible stack overlap between threads if it is run
> not from main thread. Is it your concern too ?

For non-main threads we should not use rlimit. the rlimit stuff should
be in if (something) so that it is triggered for main thread only.

For main thread the ulimit should be safe. Kernel should create
mappings in such a way that they don't overlap. In fact, glibc uses
getrlimit(RLIMIT_STACK) inside pthread_getattr_np call to calculate
base exactly this way.

/Timo



---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Reply to thread Export thread (mbox)