Added config check for coroutine_stack_size

[ilya-maltsev]

No description provided.

[ilya-maltsev]

Looks like it's time to renew the certificates

[reshke]

I like this idea to check coroutine_stack_size param, actually, LDAP is not only the case here.
For PAM auth i used to face stack overflow problems too.

[pmwkaa]

I think we should investigate the idea of using mmap(MAP_GROWSDOWN) for stack instead of relying on the fixed stack size. I don't have much time to take a look, but I think it is doable

[ilya-maltsev]

simply adding the MAP_GROWSDOWN flag into mmap doesn't help

[pmwkaa]

This is not how it works, you should find a working example how to implement it, it is more complex

[ilya-maltsev]

Some people say: "Trying to rely on the MAP_GROWSDOWN flag is unreliable and dangerous because it cannot protect you against mmap creating a new mapping just adjacent to your stack, which will then get clobbered."
Also in many of last commits on github people removing using of MAP_GROWSDOWN.
May be I don't undestend concept, but for me it is not best idea for finding optimal stack size.
I think it's must be more simply and careful.

[pmwkaa]

The idea there is that you allow to grow stack automatically up to some adequate limit. It is better to find an implementation, how people are using it in their projects. It is not necessary about the GROWSDOWN flag, we need to allocate memory for stack usage (MAP_STACK) with guard page support. The direction of stack grows itself might be different depending on the architecture.

This is more like a suggestion to research this field, if this can be done efficiently in Odyssey case. Maybe there is some lower limit needs to be allocated each time which is quite high, I am not sure. I believe it is worth to investigate anyway.

[ilya-maltsev]

Great idea, but I think dynamic memory allocation is not in best practices for high load projects, anyway for C-based, for example in nginx if you create many virtual servers more than placed in memory you get an error:
Restarting nginx: nginx: [emerg] could not build the server_names_hash, you should increase either server_names_hash_max_size: 256 or server_names_hash_bucket_size: 64.
although it would seem, why not use a predetermined limit for some simple name table hash?
but seems not worth the trouble

[ilya-maltsev]

@pmwkaa
is it possible to replace mmap with malloc at mm_contextstack_create ?
in my local_build this approach solved the problem...

[pmwkaa]

Yes, heap memory is essentially the same. Not sure how you would solve the problem with dynamic resizing though. I don't think you can realloc coroutine stack, because it could change base address and mess with the pointers allocated on stack.

[ilya-maltsev]

Problem does not in dynamic resizing, its was described in issue
And also when PAM auth was used (as reshke said).
As a result, if use malloc instead of mmap odyssey does not crash with serfault when using too small values of the coroutine_stack_size

[pmwkaa]

It probably rewrite memory after it, you should check memory santizer to make such it is not happening. Also I recommend enable malloc debuggning env variables to catch those errors

[ilya-maltsev]

you goddamn right)

[ilya-maltsev]

@pmwkaa
As a result for odyssey correct work with ldap_auth coroutine_stack_size must be more 8 and for pam_auth coroutine_stack_size must be more 128
Sorry but I can't see a way to solve this problem with dynamic memory allocation, because the MAP_GROWSDOWN flag of mmap allows the stack to grow downward into the new page, but it does not automatically allocate the new page and using malloc instead of mmap doesn't solve the problem correctly either.

[reshke]

Looks like it's time to renew the certificates

can you create separate PR for this change?

[ilya-maltsev]

Looks like it's time to renew the certificates

can you create separate PR for this change?

#492

[rkhapov]

seemd like this pr is abandoned