274450 - Linux now supports setstacksize

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Mathieu Poumeyrol]

I am questioning the usefullness of the #ifndef LINUX surrounding the call to
pthread_attr_setstacksize in ptthread.c .
While debugging a heavy multithreaded application, I found out that
pthread_attr_setstatcksize actualy exists on at least recent debian and redhat.
And that it seams to do what it is supposed to do. I don't when it started to
work, but are we sure we are not building a workaround around something which is
no longer here ?
I had to remove the #ifndef to get my application working fine. I can go on
patching nsprpub, but it is a real pain...

Comment 1

[Wan-Teh Chang]

You said:
> I had to remove the #ifndef to get my application working fine.

Does this mean your application requires setting the
threads' stack size (to a smaller value)?

Comment 2

[Mathieu Poumeyrol]

(In reply to comment #1)
> Does this mean your application requires setting the
> threads' stack size (to a smaller value)?

Precisely.

(I know an application should not start "that many" threads, so no need to waste
your time on the lecture if you don't want to change the current behaviour ;) .)

Comment 3

[Wan-Teh Chang]

Attachment: Proposed patch

Comment 4

[Wan-Teh Chang]

I confirmed that Red Hat Linux Advanced Server 2.1
has pthread_attr_setstacksize.  So I checked in the
patch on the NSPR tip (NSPR 4.6) and the
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.8 Beta 2).

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This caused bug 285330.

Comment 6

[Wan-Teh Chang]

64K default stack size is too small for Linux.

I will attach a new patch.

Comment 7

[Wan-Teh Chang]

Attachment: Proposed patch v2

In this patch, we abandon NSPR's old policy of
having a cross-platform default thread stack size.
Instead, we define NSPR's default thread stack
size to be the native thread library's default
stack size.

It is hard to determine what the right default
stack size is, so I propose we defer this decision
to the native thread library's implementors.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 176775 [details] [diff] [review]
Proposed patch v2

Looks reasonable.  r=bzbarsky

Comment 9

[Wan-Teh Chang]

Comment on attachment 176775 [details] [diff] [review]
Proposed patch v2

I checked in this patch on the NSPR tip (NSPR 4.6)
and NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.8 Beta 2).

Looking at the history of the code, I found that the
_MD_MINIMUM_STACK_SIZE code was added to fix the same
problem on OpenVMS (bug 69496) and AIX (bug 183907).
Since we have a new fix for the "default stack size too
small" problem, we should be able to remove the
_MD_MINIMUM_STACK_SIZE code from ptthread.c.  But doing
that has a higher risk because some code may be passing
a nonzero stackSize argument to PR_CreateThread and
inadvertently depend on the _MD_MINIMUM_STACK_SIZE code
to protect them.

Comment 10

[Wan-Teh Chang]

Marked the bug fixed again.