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

[Andrew John Hughes]

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.

Is there a roadmap for allowing external access to JPRT so those with
commit access like me and Andrew can push HotSpot patches ourselves?

> 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,
-- 
Andrew :-)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net

PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8