<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <meta content="text/html;charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
<br>
<br>
On 03/18/10 14:25, Andrew John Hughes wrote:
<blockquote
 cite="mid:17c6771e1003181125q7062d0fcod77a00afdf495352@mail.gmail.com"
 type="cite">
  <pre wrap="">On 18 March 2010 17:48, Coleen Phillimore - Sun Microsystems
<a class="moz-txt-link-rfc2396E" href="mailto:Coleen.Phillimore@sun.com">&lt;Coleen.Phillimore@sun.com&gt;</a> wrote:
  </pre>
  <blockquote type="cite">
    <pre wrap="">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: <a class="moz-txt-link-abbreviated" href="mailto:aph@redhat.com">aph@redhat.com</a>
Reviewed-by: coleenp, aph &lt;other reviewers???&gt;

It would be nice if we had one more reviewer....

    </pre>
  </blockquote>
  <pre wrap=""><!---->
Looks good to me, so feel free to add me to the list.
  </pre>
</blockquote>
Thanks!<br>
<blockquote
 cite="mid:17c6771e1003181125q7062d0fcod77a00afdf495352@mail.gmail.com"
 type="cite">
  <pre wrap="">
Is there a roadmap for allowing external access to JPRT so those with
commit access like me and Andrew can push HotSpot patches ourselves?

  </pre>
</blockquote>
I have no idea about that.<br>
<br>
Thanks,<br>
Coleen<br>
<blockquote
 cite="mid:17c6771e1003181125q7062d0fcod77a00afdf495352@mail.gmail.com"
 type="cite">
  <pre wrap=""></pre>
  <blockquote type="cite">
    <pre wrap="">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 <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~coleenp/6936168/">http://cr.openjdk.java.net/~coleenp/6936168/</a>
bug link at <a class="moz-txt-link-freetext" href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6936168">http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6936168</a>

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 <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~coleenp/6929067/">http://cr.openjdk.java.net/~coleenp/6929067/</a>
bug link at <a class="moz-txt-link-freetext" href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6929067">http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6929067</a>

Andrew, please have a look since you're the contributor.



That's OK, but you don't need SYS_gettid.
Please look at
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~aph/6929067-jdk7-webrev-4/hotspot.patch">http://cr.openjdk.java.net/~aph/6929067-jdk7-webrev-4/hotspot.patch</a>
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.




    </pre>
  </blockquote>
  <pre wrap=""><!---->

Thanks,
  </pre>
</blockquote>
</body>
</html>