httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: svn commit: r699481 - /httpd/httpd/trunk/server/mpm/winnt/child.c
Date Sat, 27 Sep 2008 20:14:25 GMT
Mladen Turk wrote:
> wrowe@apache.org wrote:
>> Author: wrowe
>> Date: Fri Sep 26 13:15:10 2008
>> New Revision: 699481
>>
>> URL: http://svn.apache.org/viewvc?rev=699481&view=rev
>> Log:
>> Reimplement ThreadStackSize to behave as on unix for any
>> Windows 2003/2008 (XP/Vista) servers.  Virtual allocations
>> will only consume pages once referenced, while the page
>> alignment will vary by ThreadStackSize setting so that the
>> maximum number of threads and minimum stack VM profile will
>> be wasted.
>>
>> -            child_handles[i] = (HANDLE) _beginthreadex(
> 
>> +            child_handles[i] = CreateThread(NULL, ap_thread_stacksize,
> 
> Why did you change the _beginthreadex to CreateThread?
> Won't that bring back CRT local Tls problems that forced using
> _beginthreadex at the first place?

Please site the Tls problems.  There is one I can think of, which is
the behavior of the locale (inherit from parent v.s. inherit from global)
but that's not even a portable contract between unix and windows modules.
Otherwise, TLS is allocated on first-reference.

_beginthreadex() is further dangerous because returning from that thread,
before return, the handle is *closed*.  Elsewhere we are testing those
handles to see if the thread has terminated, potentially what it's return
code is.  So closing the handle before we do is invalid behavior.

> Also I have concerns with chosen 64K thread stack size for
> some other threads in this patch series. Perhaps a 256K will
> be more safe (particularly for winnt_accept), because 64K on
> stack overflow OS will allocate 1M actually, so this either needs to
> get carefully calculated or traced by some tool, cause larger
> initial commit size might eventually use less memory.

Are you sure?  Please validate your assumptions with actual testing
before postulating; the original behavior and assumptions of the entire
ThreadStackSize behavior on windows was so off base as to be laughable.
These changes are all based on direct observation, on Windows XP (2003)
to be confirmed on 2008 before I propose for backport.

Can you look at those threads and please justify a larger stack size
rather than just kicking them up for the sake of it?  Based on rev 88498
I'm pretty sure that several choices were made without any real basis
to get the code adjusted, and they never were revisited.


Mime
View raw message