[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


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #57735|review?                     |review-
               Flag|                            |




--- Comment #4 from Alexey Proskuryakov <ap at webkit.org>  2010-06-03 11:18:41 PST ---
(From update of attachment 57735)
> This might not be enough.

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?

> +        Use macro ENABLE_THREAD_STACK_SIZE_CHECK to get this code change compiled so it will not affect most platforms. And macro MINIMAL_THREAD_STACK_SIZE is used to define the minimal thread stack size. This code change will only use MINIMAL_THREAD_STACK_SIZE if the default stack size is smaller than MINIMAL_THREAD_STACK_SIZE.

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

+    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?

-    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.

+    ASSERT(!pthread_attr_init(&attr));

This is wrong - this code won't be compiled in release builds. Same mistake is repeated elsewhere.

+    if (!threadHandle)
+        return 0;
+
+    return establishIdentifierForPthreadHandle(threadHandle);

No need to duplicate the common part of the function.

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