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

[Coleen Phillimore - Sun Microsystems]

On 03/18/10 14:25, Andrew John Hughes wrote:
> On 18 March 2010 17:48, Coleen Phillimore - Sun Microsystems
> <Coleen.Phillimore at sun.com> wrote:
>   
>> Andrew, Since I missed the contributed by in the last putback, this one will
>> look like this:
>>
>> 6936168: Recent fix for unmapping stack guard pages doesn't close
>> /proc/self/maps
>> Summary: Add close to returns (fix for 6929067 also contributed by aph)
>> Contributed-by: aph at redhat.com
>> Reviewed-by: coleenp, aph <other reviewers???>
>>
>> It would be nice if we had one more reviewer....
>>
>>     
>
> Looks good to me, so feel free to add me to the list.
>   
Thanks!
> Is there a roadmap for allowing external access to JPRT so those with
> commit access like me and Andrew can push HotSpot patches ourselves?
>
>   
I have no idea about that.

Thanks,
Coleen
>> Thanks,
>> Coleen
>>
>> On 03/18/10 13:46, Andrew Haley wrote:
>>
>> 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.
>>
>>
>>
>>
>>     
>
>
> Thanks,
>   
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-dev/attachments/20100318/fbbb3d16/attachment.html