[Webkit-unassigned] [Bug 40103] ThreadingPthread create thread with default stack size which might not be enough

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40103





--- Comment #5 from Lyon Chen <lyon.chen at torchmobile.com>  2010-06-03 11:31:01 PST ---
(In reply to comment #4)
> What exactly the bug is? Secondary thread stack size is also smaller on Mac OS X, and some may say it's "not enough".
> 
> From all I can see, this patch just adds mysterious code that's not enabled on any platform. Some questions the code and/or ChangeLog should help to answer are:
> - is this code still needed, or can it be removed?
> - do I want this on my platform?
> - if yes, how do I choose minimal stack size?

This patch is needed for any platform that use pthread and its pthread_create() uses too small stack size that it will cause stack overflow. For me 64 * 1024 is fine so far.

> This line is too long to fit in a reasonably sized window. Please stick to how other ChangeLog entries are formatted.
> 

OK, will reformat the change log message.

> +    if (stackSize < MINIMAL_THREAD_STACK_SIZE) {
> 
> It's best to include the measurement unit in constant name to avoid confusion. Maybe MINIMAL_THREAD_STACK_BYTES?

Yeah, this is clearer.

> 
> -    if (result == 0)
> +    if (!result)
>          return true;
> 
> I don't really think that changes like this are an improvement. And in any case, it's best to not make lots of unrelated coding style changes in patches for real bugs.

This is not what I planned to do, but without this change the coding style checked failed my patch, as what it did to my first patch.

> 
> +    ASSERT(!pthread_attr_init(&attr));
> 
> This is wrong - this code won't be compiled in release builds. Same mistake is repeated elsewhere.

Yeah, you are right, forgot that ASSERT won't work on release build.

> No need to duplicate the common part of the function.

OK, I can change this too.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.