Request for approval: 6929067: Stack guard pages should be removed when thread is detached

[Andrew Haley]

Yes, thanks indeed.

Andrew.


On 03/18/2010 05:35 PM, Coleen Phillimore - Sun Microsystems wrote:
> 
> I filed bug  6936168 and have a repository with 3 additional fcloses()
> in them for the returns after the file is opened.  See:
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/6936168/
> bug link at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6936168
> 
> It's running our tests that failed now.  Please review this change and
> I'll push it back before it gets to the main hotspot repository (it's
> still in hotspot_rt right now).
> 
> Thanks for reporting this.  This would have taken us a while to
> diagnose.  Sorry for the crummy code review the first time around.
> 
> Coleen
> 
> On 03/18/10 06:05, Andrew Haley wrote:
>> On 03/18/2010 09:10 AM, Andreas Kohn wrote:
>>  
>>> On Fri, 2010-03-12 at 09:44 +0000, Andrew Haley wrote:
>>>    
>>>> On 03/11/2010 09:06 PM, Coleen Phillimore wrote:
>>>>      
>>>>> I've added the test to the changeset and a script to run in our
>>>>> harness.
>>>>>
>>>>> Also in os_linux.cpp, I changed the SYS_gettid call to go through our
>>>>> os::Linux::gettid() because on at least one linux, syscall() returns a
>>>>> long int which gets a compilation warning with %d.
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/6929067/
>>>>> bug link at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6929067
>>>>>
>>>>> Andrew, please have a look since you're the contributor.
>>>>>         
>>>> That's OK, but you don't need SYS_gettid.
>>>> Please look at
>>>> http://cr.openjdk.java.net/~aph/6929067-jdk7-webrev-4/hotspot.patch
>>>> I changed to "/proc/self/maps", as you requested.  I think this is
>>>> better.
>>>>
>>>> The copy of my webrev to cr.openjdk.java.net failed for some reason
>>>> I don't understand.
>>>>       
>>> With this change I seem to hit the limit on the number of open files.
>>> Looking through it, shouldn't get_stack_bounds() close the FILE* it
>>> opened?
>>>     
>>
>> Oh, how stupid of me!  If this were gcc I'd just push a fix
>> immediately as obvious/trivial, but I think we need a bug opened to
>> push the change.
>>
>> (BTW, this happened because of a mistake translating the patch I wrote
>> from using the C++ library into C.  The original patch used an
>> fstream, whose destructor closes the file.  When I did the translation
>> I missed the fact that I had to close the file manually.)
>>
>> Andrew.
>>   
>